Global bindings are not immutable, contrary to documentation
geoffreyvanwyk opened this issue ยท 28 comments
Q | A |
---|---|
Phel version | v0.14 |
PHP version | 8.3.7 |
Summary
The documentation states:
(def name meta? value)
This special form binds a value to a global symbol. A definition cannot be redefined at a later point.
Current behavior
A definition can be redefined without an exception being raised.
How to reproduce
Enter the following expressions into the REPL.
(def my-name "PHP")
(def my-name "Phel")
Expected behavior
An exception is raised with a message that my-name
is already bound.
This is a known feature. On the REPL, defn
allows you do redefine the same global binding - similar as in Clojure.
I am not sure if we want to change this behaviour; I would rather suggest update the docs to add this knowledge instead.
OK for you, @jenshaase?
This is a known feature. On the REPL,
defn
allows you do redefine the same global binding - similar as in Clojure.
In this case, it is just def
, not defn
.
Hi, I'm @t-matsudate.
I consider this occurs because the def
symbol analyzer allows any duplicates of namespace/name pairs.
Therefore, I'd like to propose following code to resolve this issue.
// DefSymbol.php
if (!$this->analyzer->hasDefinition($namespace, $nameSymbol->getName())) {
$this->analyzer->addDefinition($namespace, $nameSymbol);
} else {
throw AnalyzerException::definitionDuplication(...);
}
However I've not been sure specification of the Phel yet.
So I wish to hear everyone's consideration of this issue.
Oh, good spot. Now I get what you mean. Do you mind opening a PR with your proposal, I think that should be correct in order to preserving immutability of the definitions. @jenshaase, was it some reason why we have this behaviour that I cannot recall? Or we just simply missed it?
@Chemaclass Okay. please wait some moment. I'll create PR after implementing it with tests.
I sometimes use REPL to test the behavior of a function.
If def
or defn
cannot be redefined, I have to rerun REPL many times.
Wouldn't it be more convenient to be able to redefine def
and defn
at least when running REPL?
@smeghead Although this is case in the Clojure, that has several way to redefine vars instead of reusing def
:
This changes root binding of its var while keeping the namespace/name symbol.
However we should use this in some limited case because this FORCIBLY changes root bindging. this makes callers confused until with-redefs
's scope got exited.
This is same as with-redefs
except this uses any updater function.
- using dynamic vars
This is the thread local binding in the Clojure.
Probably this is the most safe way.
Otherwise the Clojure requires us that switchs current namespace.
@Chemaclass Yes. And also I'm thinking its rule shouldn't easily be changed between repl and run.
I've written following code to resolve this issue:
// DefSymbol.php
if (!$this->analizer->hasDefinition($namespace, $nameSymbol->getName())) {
$this->analyzer->addDefinition($namespace, $nameSymbol);
} else {
throw AnalyzerException::definitionDuplication($list, $namespace, $nameSymbol);
}
// AnalyzerException.php
public static function definitionDuplication(
PersistentListInterface $list,
string $namespace,
Symbol $nameSymbol
): self {
return self::withLocation(
sprintf(
'Var "%s"\\"%s" is already defined.',
$namespace,
$nameSymbol->getName(),
),
$list,
);
}
Then php-cs-fixer noticed above simple file changes as an error.
~/phel-lang$ composer test
> vendor/bin/psalm --clear-cache
Cache directory deleted
> vendor/bin/phpstan clear-result-cache
Note: Using configuration file /home/runner/phel-lang/phpstan.neon.
Result cache cleared from directory:
/tmp/phpstan
> ./vendor/bin/php-cs-fixer fix --dry-run
PHP CS Fixer 3.58.1 7th Gear by Fabien Potencier, Dariusz Ruminski and contributors.
PHP runtime: 8.3.0
Running analysis on 1 core sequentially.
You can enable parallel runner and speed up the analysis! Please see usage docs for more information.
Loaded config default from "/home/runner/phel-lang/.php-cs-fixer.dist.php".
Using cache file ".php-cs-fixer.cache".
592/592 [โโโโโโโโโโโโโโโโโโโโโโโโโโโโ] 100%
1) src/php/Compiler/Domain/Analyzer/Exceptions/AnalyzerException.php
Found 1 of 592 files that can be fixed in 0.160 seconds, 20.00 MB memory used
Script ./vendor/bin/php-cs-fixer fix --dry-run handling the csrun event returned with error code 8
Script @csrun was called via test-quality
Script @test-quality was called via test-all
Script @test-all was called via test
Could you tell me how do I cope with this thing?
Have you tried "composer fix" first?
Sorry, I was forgetting it and ran composer fix
.
Then I got the message to mean of no error from php-cs-fixer. ๐
After above, I added following fixes:
// AnalyzerInterface.php
public function hasDefinition(string $ns, Symbol $name): bool;
// Analyzer.php
public function hasDefinition(string $ns, Symbol $name): bool
{
return $this->globalEnvironment->hasDefinition($ns, $name);
}
And I ran composer test
once more. Then I got following errors:
Cache directory deleted
Result cache cleared from directory:
/tmp/phpstan
Found 0 of 592 files that can be fixed in 0.027 seconds, 20.00 MB memory used
------------------------------
๏ฟฝ[42;2m ๏ฟฝ[0m
๏ฟฝ[42;30;2m No errors found! ๏ฟฝ[0m
๏ฟฝ[42;2m ๏ฟฝ[0m
------------------------------
Psalm can automatically fix 13 of these issues.
Run Psalm again with
๏ฟฝ[30;48;5;195m--alter --issues=MissingParamType --dry-run๏ฟฝ[0m
to see what it can fix.
------------------------------
Checks took 10.32 seconds and used 144.240MB of memory
Psalm was able to infer types for 97.8284% of the codebase
[OK] No errors
[OK] Rector is done!
PHPUnit 9.6.19 by Sebastian Bergmann and contributors.
Runtime: PHP 8.3.0
Configuration: /home/runner/phel-lang/phpunit.xml.dist
Random Seed: 1718699591
...............................FEEEEEEEEE...RRREFFEE........... 63 / 914 ( 6%)
....................................................F.....SFS.F 126 / 914 ( 13%)
[2024-06-18T08:33:12+00:00] Error Unknown(-1) found!
message: "file_get_contents(/home/runner/phel-lang/tests/php/Integration/Build/Command/out/main.php): Failed to open stream: No such file or directory"
file: /home/runner/phel-lang/tests/php/Integration/Build/Command/BuildCommandTest.php:107
context: {"errno":2}
............................................................... 189 / 914 ( 20%)
............................................................... 252 / 914 ( 27%)
............................................................... 315 / 914 ( 34%)
............................................................... 378 / 914 ( 41%)
................................................Phel\Compiler\Domain\Analyzer\Exceptions\AnalyzerException: .
............... 441 / 914 ( 48%)
............................................................... 504 / 914 ( 55%)
............................................................... 567 / 914 ( 62%)
............................................................... 630 / 914 ( 68%)
............................................................... 693 / 914 ( 75%)
............................................................... 756 / 914 ( 82%)
...........................................
Time: 00:01.507, Memory: 28.00 MB
There were 12 errors:
1) PhelTest\Integration\Run\Command\Repl\ReplCommandTest::test_integration_with_core_lib with data set "require-namespace.test" ('phel:1> (require phel\html :a...div>\n', PhelTest\Integration\Run\Command\Repl\InputLine Object (...), PhelTest\Integration\Run\Command\Repl\InputLine Object (...))
Phel\Compiler\Domain\Exceptions\CompilerException: Var "phel\core"\"*ns*" is already defined.
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:97
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:68
/home/runner/phel-lang/src/php/Compiler/CompilerFacade.php:57
/home/runner/phel-lang/src/php/Build/Application/FileEvaluator.php:25
/home/runner/phel-lang/src/php/Build/BuildFacade.php:102
/home/runner/phel-lang/src/php/Run/RunFacade.php:69
/home/runner/phel-lang/src/php/Run/Infrastructure/Command/ReplCommand.php:128
/home/runner/phel-lang/src/php/Run/Infrastructure/Command/ReplCommand.php:97
/home/runner/phel-lang/vendor/symfony/console/Command/Command.php:279
/home/runner/phel-lang/tests/php/Integration/Run/Command/Repl/ReplCommandTest.php:74
2) PhelTest\Integration\Run\Command\Repl\ReplCommandTest::test_integration with data set "undefined-symbol.test" ('phel:1> (def my-fn (fn []\n.....^^\n\n', PhelTest\Integration\Run\Command\Repl\InputLine Object (...), PhelTest\Integration\Run\Command\Repl\InputLine Object (...))
Phel\Compiler\Domain\Exceptions\CompilerException: Var "phel\core"\"*ns*" is already defined.
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:97
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:68
/home/runner/phel-lang/src/php/Compiler/CompilerFacade.php:57
/home/runner/phel-lang/src/php/Build/Application/FileEvaluator.php:25
/home/runner/phel-lang/src/php/Build/BuildFacade.php:102
/home/runner/phel-lang/src/php/Run/RunFacade.php:69
/home/runner/phel-lang/src/php/Run/Infrastructure/Command/ReplCommand.php:128
/home/runner/phel-lang/src/php/Run/Infrastructure/Command/ReplCommand.php:97
/home/runner/phel-lang/vendor/symfony/console/Command/Command.php:279
/home/runner/phel-lang/tests/php/Integration/Run/Command/Repl/ReplCommandTest.php:51
3) PhelTest\Integration\Run\Command\Repl\ReplCommandTest::test_integration with data set "unterminated-list-multiline-with-empty-lines.test" ('phel:1> (php/+ 1\n....:2>\n..... ^\n', PhelTest\Integration\Run\Command\Repl\InputLine Object (...), PhelTest\Integration\Run\Command\Repl\InputLine Object (...), PhelTest\Integration\Run\Command\Repl\InputLine Object (...), PhelTest\Integration\Run\Command\Repl\InputLine Object (...), PhelTest\Integration\Run\Command\Repl\InputLine Object (...))
Phel\Compiler\Domain\Exceptions\CompilerException: Var "phel\core"\"*ns*" is already defined.
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:97
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:68
/home/runner/phel-lang/src/php/Compiler/CompilerFacade.php:57
/home/runner/phel-lang/src/php/Build/Application/FileEvaluator.php:25
/home/runner/phel-lang/src/php/Build/BuildFacade.php:102
/home/runner/phel-lang/src/php/Run/RunFacade.php:69
/home/runner/phel-lang/src/php/Run/Infrastructure/Command/ReplCommand.php:128
/home/runner/phel-lang/src/php/Run/Infrastructure/Command/ReplCommand.php:97
/home/runner/phel-lang/vendor/symfony/console/Command/Command.php:279
/home/runner/phel-lang/tests/php/Integration/Run/Command/Repl/ReplCommandTest.php:51
4) PhelTest\Integration\Run\Command\Repl\ReplCommandTest::test_integration with data set "simple-add.test" ('phel:1> (php/+ 1 1)\n2\n', PhelTest\Integration\Run\Command\Repl\InputLine Object (...))
Phel\Compiler\Domain\Exceptions\CompilerException: Var "phel\core"\"*ns*" is already defined.
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:97
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:68
/home/runner/phel-lang/src/php/Compiler/CompilerFacade.php:57
/home/runner/phel-lang/src/php/Build/Application/FileEvaluator.php:25
/home/runner/phel-lang/src/php/Build/BuildFacade.php:102
/home/runner/phel-lang/src/php/Run/RunFacade.php:69
/home/runner/phel-lang/src/php/Run/Infrastructure/Command/ReplCommand.php:128
/home/runner/phel-lang/src/php/Run/Infrastructure/Command/ReplCommand.php:97
/home/runner/phel-lang/vendor/symfony/console/Command/Command.php:279
/home/runner/phel-lang/tests/php/Integration/Run/Command/Repl/ReplCommandTest.php:51
5) PhelTest\Integration\Run\Command\Repl\ReplCommandTest::test_integration with data set "simple-add-multiline.test" ('phel:1> (php/+ 1\n....:2> 2\n...)\n5\n', PhelTest\Integration\Run\Command\Repl\InputLine Object (...), PhelTest\Integration\Run\Command\Repl\InputLine Object (...), PhelTest\Integration\Run\Command\Repl\InputLine Object (...), PhelTest\Integration\Run\Command\Repl\InputLine Object (...))
Phel\Compiler\Domain\Exceptions\CompilerException: Var "phel\core"\"*ns*" is already defined.
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:97
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:68
/home/runner/phel-lang/src/php/Compiler/CompilerFacade.php:57
/home/runner/phel-lang/src/php/Build/Application/FileEvaluator.php:25
/home/runner/phel-lang/src/php/Build/BuildFacade.php:102
/home/runner/phel-lang/src/php/Run/RunFacade.php:69
/home/runner/phel-lang/src/php/Run/Infrastructure/Command/ReplCommand.php:128
/home/runner/phel-lang/src/php/Run/Infrastructure/Command/ReplCommand.php:97
/home/runner/phel-lang/vendor/symfony/console/Command/Command.php:279
/home/runner/phel-lang/tests/php/Integration/Run/Command/Repl/ReplCommandTest.php:51
6) PhelTest\Integration\Run\Command\Repl\ReplCommandTest::test_integration with data set "exit-multiline-mode-with-ctrl-d.test" ('phel:1> (php/+ 1\n....:2> 2\n...)\n3\n', PhelTest\Integration\Run\Command\Repl\InputLine Object (...), PhelTest\Integration\Run\Command\Repl\InputLine Object (...), PhelTest\Integration\Run\Command\Repl\InputLine Object (...), PhelTest\Integration\Run\Command\Repl\InputLine Object (...))
Phel\Compiler\Domain\Exceptions\CompilerException: Var "phel\core"\"*ns*" is already defined.
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:97
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:68
/home/runner/phel-lang/src/php/Compiler/CompilerFacade.php:57
/home/runner/phel-lang/src/php/Build/Application/FileEvaluator.php:25
/home/runner/phel-lang/src/php/Build/BuildFacade.php:102
/home/runner/phel-lang/src/php/Run/RunFacade.php:69
/home/runner/phel-lang/src/php/Run/Infrastructure/Command/ReplCommand.php:128
/home/runner/phel-lang/src/php/Run/Infrastructure/Command/ReplCommand.php:97
/home/runner/phel-lang/vendor/symfony/console/Command/Command.php:279
/home/runner/phel-lang/tests/php/Integration/Run/Command/Repl/ReplCommandTest.php:51
7) PhelTest\Integration\Run\Command\Repl\ReplCommandTest::test_integration with data set "unterminated-list-multiline.test" ('phel:1> (php/+ 1\n....:2> 2\n... ^\n', PhelTest\Integration\Run\Command\Repl\InputLine Object (...), PhelTest\Integration\Run\Command\Repl\InputLine Object (...), PhelTest\Integration\Run\Command\Repl\InputLine Object (...))
Phel\Compiler\Domain\Exceptions\CompilerException: Var "phel\core"\"*ns*" is already defined.
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:97
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:68
/home/runner/phel-lang/src/php/Compiler/CompilerFacade.php:57
/home/runner/phel-lang/src/php/Build/Application/FileEvaluator.php:25
/home/runner/phel-lang/src/php/Build/BuildFacade.php:102
/home/runner/phel-lang/src/php/Run/RunFacade.php:69
/home/runner/phel-lang/src/php/Run/Infrastructure/Command/ReplCommand.php:128
/home/runner/phel-lang/src/php/Run/Infrastructure/Command/ReplCommand.php:97
/home/runner/phel-lang/vendor/symfony/console/Command/Command.php:279
/home/runner/phel-lang/tests/php/Integration/Run/Command/Repl/ReplCommandTest.php:51
8) PhelTest\Integration\Phel\PhelTest::test_globals_argv_as_string_via_run
Phel\Compiler\Domain\Exceptions\CompilerException: Var "phel\core"\"*ns*" is already defined.
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:97
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:68
/home/runner/phel-lang/src/php/Compiler/CompilerFacade.php:57
/home/runner/phel-lang/src/php/Build/Application/FileEvaluator.php:25
/home/runner/phel-lang/src/php/Build/BuildFacade.php:102
/home/runner/phel-lang/src/php/Run/Application/NamespaceRunner.php:32
/home/runner/phel-lang/src/php/Run/RunFacade.php:23
/home/runner/phel-lang/src/php/Phel.php:38
/home/runner/phel-lang/tests/php/Integration/Phel/PhelTest.php:26
9) PhelTest\Integration\Phel\PhelTest::test_globals_argv_as_array_via_run
Phel\Compiler\Domain\Exceptions\CompilerException: Var "phel\core"\"*ns*" is already defined.
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:97
/home/runner/phel-lang/src/php/Compiler/Application/EvalCompiler.php:68
/home/runner/phel-lang/src/php/Compiler/CompilerFacade.php:57
/home/runner/phel-lang/src/php/Build/Application/FileEvaluator.php:25
/home/runner/phel-lang/src/php/Build/BuildFacade.php:102
/home/runner/phel-lang/src/php/Run/Application/NamespaceRunner.php:32
/home/runner/phel-lang/src/php/Run/RunFacade.php:23
/home/runner/phel-lang/src/php/Phel.php:38
/home/runner/phel-lang/tests/php/Integration/Phel/PhelTest.php:34
10) PhelTest\Integration\IntegrationTest::test_integration
Phel\Compiler\Domain\Exceptions\CompilerException: Var "phel\core"\"*ns*" is already defined. in /home/runner/phel-lang/src/php/Compiler/Application/CodeCompiler.php:102
Stack trace:
#0 /home/runner/phel-lang/src/php/Compiler/Application/CodeCompiler.php(69): Phel\Compiler\Application\CodeCompiler->analyze(Object(Phel\Compiler\Domain\Parser\ReadModel\ReaderResult))
#1 /home/runner/phel-lang/src/php/Compiler/CompilerFacade.php(88): Phel\Compiler\Application\CodeCompiler->compileString('(ns phel\\core\n ...', Object(Phel\Compiler\Infrastructure\CompileOptions))
#2 /home/runner/phel-lang/src/php/Build/Application/FileCompiler.php(33): Phel\Compiler\CompilerFacade->compile('(ns phel\\core\n ...', Object(Phel\Compiler\Infrastructure\CompileOptions))
#3 /home/runner/phel-lang/src/php/Build/BuildFacade.php(90): Phel\Build\Application\FileCompiler->compileFile('/home/runner/ph...', '/tmp/phel-coree...', true)
#4 /home/runner/phel-lang/tests/php/Integration/IntegrationTest.php(31): Phel\Build\BuildFacade->compileFile('/home/runner/ph...', '/tmp/phel-coree...')
#5 /home/runner/phel-lang/vendor/phpunit/phpunit/src/Framework/TestSuite.php(629): PhelTest\Integration\IntegrationTest::setUpBeforeClass()
#6 /home/runner/phel-lang/vendor/phpunit/phpunit/src/Framework/TestSuite.php(685): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
#7 /home/runner/phel-lang/vendor/phpunit/phpunit/src/Framework/TestSuite.php(685): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
#8 /home/runner/phel-lang/vendor/phpunit/phpunit/src/TextUI/TestRunner.php(651): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
#9 /home/runner/phel-lang/vendor/phpunit/phpunit/src/TextUI/Command.php(146): PHPUnit\TextUI\TestRunner->run(Object(PHPUnit\Framework\TestSuite), Array, Array, true)
#10 /home/runner/phel-lang/vendor/phpunit/phpunit/src/TextUI/Command.php(99): PHPUnit\TextUI\Command->run(Array, true)
#11 /home/runner/phel-lang/vendor/phpunit/phpunit/phpunit(107): PHPUnit\TextUI\Command::main()
#12 /home/runner/phel-lang/vendor/bin/phpunit(122): include('/home/runner/ph...')
#13 {main}
11) PhelTest\Integration\Interop\CallPhelTest::test_call_odd
Phel\Compiler\Domain\Exceptions\CompilerException: Var "phel\core"\"*ns*" is already defined.
/home/runner/phel-lang/src/php/Compiler/Application/CodeCompiler.php:102
/home/runner/phel-lang/src/php/Compiler/Application/CodeCompiler.php:69
/home/runner/phel-lang/src/php/Compiler/CompilerFacade.php:88
/home/runner/phel-lang/src/php/Build/Application/FileCompiler.php:33
/home/runner/phel-lang/src/php/Build/BuildFacade.php:90
/home/runner/phel-lang/tests/php/Integration/Interop/CallPhelTest.php:22
12) PhelTest\Integration\Interop\CallPhelTest::test_call_print_str
Phel\Compiler\Domain\Exceptions\CompilerException: Var "phel\core"\"*ns*" is already defined.
/home/runner/phel-lang/src/php/Compiler/Application/CodeCompiler.php:102
/home/runner/phel-lang/src/php/Compiler/Application/CodeCompiler.php:69
/home/runner/phel-lang/src/php/Compiler/CompilerFacade.php:88
/home/runner/phel-lang/src/php/Build/Application/FileCompiler.php:33
/home/runner/phel-lang/src/php/Build/BuildFacade.php:90
/home/runner/phel-lang/tests/php/Integration/Interop/CallPhelTest.php:22
--
There were 6 failures:
1) PhelTest\Integration\Interop\Command\Export\ExportCommandTest::test_export_command_multiple
Failed asserting that file "/home/runner/phel-lang/tests/php/Integration/Interop/Command/Export/PhelGenerated/TestCmdExportMultiple/Adder.php" exists.
/home/runner/phel-lang/tests/php/Integration/Interop/Command/Export/ExportCommandTest.php:42
2) PhelTest\Integration\Run\Command\Test\TestCommandProjectSuccess\TestCommandProjectSuccessTest::test_all_in_project
Failed asserting that '๏ฟฝ[33;34mVar "phel\core"\"*ns*" is already defined.๏ฟฝ[0m\n
in /home/runner/phel-lang/src/phel/core.phel:38\n
\n
38| (def *ns*\n
39| "Returns the namespace in the current scope."\n
40| "\\phel\\core")\n
\n
' matches PCRE pattern "/.*Total: 2.*/".
3) PhelTest\Integration\Run\Command\Test\TestCommandProjectSuccess\TestCommandProjectSuccessTest::test_one_file_in_project
Failed asserting that '๏ฟฝ[33;34mVar "phel\core"\"*ns*" is already defined.๏ฟฝ[0m\n
in /home/runner/phel-lang/src/phel/core.phel:38\n
\n
38| (def *ns*\n
39| "Returns the namespace in the current scope."\n
40| "\\phel\\core")\n
\n
' matches PCRE pattern "/.*Total: 1.*/".
4) PhelTest\Integration\Run\Command\Test\TestCommandProjectFailure\TestCommandProjectFailureTest::test_all_in_failed_project
Failed asserting that '๏ฟฝ[33;34mVar "phel\core"\"*ns*" is already defined.๏ฟฝ[0m\n
in /home/runner/phel-lang/src/phel/core.phel:38\n
\n
38| (def *ns*\n
39| "Returns the namespace in the current scope."\n
40| "\\phel\\core")\n
\n
' matches PCRE pattern "/.*Total: 1.*/".
5) PhelTest\Integration\Build\Command\BuildCommandTest::test_build_project
Failed asserting that '' matches PCRE pattern "/This is printed from no-cache.phel/".
/home/runner/phel-lang/tests/php/Integration/Build/Command/BuildCommandTest.php:47
6) PhelTest\Integration\Build\Command\BuildCommandTest::test_out_main_file
Failed asserting that false is identical to '<?php declare(strict_types=1);\n
\n
require_once dirname(__DIR__) . "/vendor/autoload.php";\n
\n
$compiledFile = __DIR__ . "/test_ns/hello.php";\n
\n
require_once $compiledFile;'.
/home/runner/phel-lang/tests/php/Integration/Build/Command/BuildCommandTest.php:117
--
There were 3 risky tests:
1) PhelTest\Integration\Run\Command\Run\RunCommandTest::test_file_not_found
Test code or tested code did not (only) close its own output buffers
2) PhelTest\Integration\Run\Command\Run\RunCommandTest::test_run_by_filename
Test code or tested code did not (only) close its own output buffers
3) PhelTest\Integration\Run\Command\Run\RunCommandTest::test_run_by_namespace
Test code or tested code did not (only) close its own output buffers
--
There were 2 skipped tests:
1) PhelTest\Integration\Api\ApiFacadeTest::test_number_of_grouped_functions
Useful to debug the facade, but useless to keep it in the CI
/home/runner/phel-lang/tests/php/Integration/Api/ApiFacadeTest.php:23
2) PhelTest\Integration\Build\Command\BuildCommandTest::test_build_project_cached
This test depends on "PhelTest\Integration\Build\Command\BuildCommandTest::test_build_project" to pass.
ERRORS!
Tests: 914, Assertions: 1285, Errors: 12, Failures: 6, Skipped: 2, Risky: 3.
This is looked seem the Phel's def
redefines *ns*
whenever we use itself.
Now I wanna confirm behavior of def
in the PHP's source code.
Could you tell me what happend at this moment in the Phel?
Do you mind creating a PR to be able to see the full diff and the CI outcome as well, @t-matsudate ? It's going to be easy to keep track on the progress and changes there
I would send PR for this but couldn't commit changes because composer test
starts at precommit timing.
What the way to skip test only this moment are there?
Or should I cancel these changes?
@t-matsudate you can commit using the --no-verify
flag to skip the git hooks
eg:
git commit -m 'your message' --no-verify
Thank you for telling good news to me!
However I've succeeded commit to perform another way.
Therefore I'll create PR for above to this repository soon.
Hello! Apologies for butting in, but wouldn't this fix entail a regression in functionality? The ability to redefine bindings is fundamental to REPL driven development in Lisps, including Clojure. It might be better to change the spec, not the implementation.
@love-your-parens
Thnks for joining this discussion.
But I wish to make your comment more clear.
It might be better to change the spec, not the implementation.
This is:
- I/We should fix Phel-side code, not PHP-side.
- I/We should revise just Phel's documentation.
Could you tell me which of above does your comment indicate?
Or do you tell about more different thing?
@t-matsudate
Sure! It's the latter: I think the current behaviour is desireable (for practical reasons), and the documentation should be revised to reflect it.
@love-your-parens
I understood your comment, then I'll swtich my work into document revisions.
Thank you for the important notice! :)
@Chemaclass @smeghead @love-your-parens @geoffreyvanwyk
I worked documentation fix as phel-lang/phel-lang.org#83.
Hi all,
my initial thoughts were exactly what the documentations says "A definition cannot be redefined at a later point.". From a theoretical point this still makes sense for me today. Redefining a definition makes program code much more hard to read because you cannot easily understand what is going on.
However, I cannot remember anymore why I didn't implemented it that way. Maybe I just forget it or I had a problem with it and wanted to fix it later.
Changing the definition in the documentation has the drawback that def
and var
are then basically the same which is also not ideal.
If someone has the time and the will to implemented the immutability of def
I would be very happy. If this is no the case I can accept that we change the documentation as request in the PR.
What do you think?
TLDR: I would consider this a "bug" that needs to be fixed rather than changing the docs.
I prefer to aim for immutability for def (except for the REPL for pragmatic reasons of quick/easy feedback on re-definitions if needed). Otherwise, as Jens says, we would lose the immutability core principle (having def and var with the same meaning...).
Hi @jenshaase @Chemaclass
This is my opinion. (again)
I believe that REPL is an important feature for the Lisp family of programming languages.
When I use phel for various programming, I basically practice Test Driven Development, but I also use REPL to redefine functions dynamically to create the desired function while checking its behavior.
If the REPL cannot be used to redefine def, it will be necessary to restart the REPL each time, which I feel will be a little less convenient.