wurstscript / WurstScript

Programming language and toolkit to create Warcraft III Maps
https://wurstlang.org
Apache License 2.0
225 stars 30 forks source link

detect cyclic dependencies caused by config packages #1001

Closed Jampi0n closed 3 years ago

Jampi0n commented 3 years ago

Fix for #990 .

There is an implicit initialization order dependency between the config and configured package: The config package is always initialized directly before the configured package. However, this dependency is not known to the wurst validator, so it is possible to create cyclic dependencies and packages will be initialized before their imports.

This PR implicitly merges the dependencies on the config package.

This can break existing and working maps. So implementing the stricter cyclic dependency check as a warning may be preferred. At least for some time, similar to globals being used above their declaration.

Frotty commented 3 years ago

Nice 👍

So implementing the stricter cyclic dependency check as a warning may be preferred

This sounds reasonable. Probably time to turn the global warning into an error now.

Jampi0n commented 3 years ago

Cyclic dependencies caused by config dependencies are now only warnings. The check runs now twice, once the old version with errors and once the new version with warnings. Reported errors are stored, so they are not reported again as a warning. I added some more functions in order to avoid code duplication.

The warning message will show that the configured and config package belong together. That should make it clear that imports from the configured package are also considered for the cyclic dependency check. Test/Test_config -> Requirement -> Test/Test_config

Frotty commented 3 years ago

Thanks 👍 We should probably change the "initlater" phrase to also include proper code organization - many ppl get some issues after heavy initlater usage, since they don't understand what they are doing.