Static Code Analysis for OpenSCAD
Locked
12
12
 
|
julianstirling wrote
> While you need to do quite a number of commands to > accomplish tasks, there is nothing stopping you breaking these up into > variable, smaller functions and modules. Actually I feel like there is a trade-off here that I struggle with. If I have a computation that I would like to embed in a function but it returns 5 values then I have to return the five values as a list without any semantic tags about what the list entries mean. This creates a different kind of complexity. Do I want to have my code littered with confusing references like return[3] and return[4]? Do I unpack the return with radius=return[0], length=return[1], scale=return[2], etc Or do I just give up on modularity and embed the calculation? I tend to find that I'm unlikely to embed a calculation unless it's actually used multiple places because the extra mess and complexity of doing so isn't worth it. -- Sent from: http://forum.openscad.org/ _______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org |
|
|
I don't know if my original post on this topic got through the mill or
not but I wanted to say the code analyzer is a GREAT addition to our toolboxes! I'm so appreciative that the author did this. I've only been coding since 1977 (both professionally and for fun!) so I don't have a lot of experience. :) Lexical analyzers are a BEAR to write, especially for such a niche tool. OpenSCAD is a marvelous program fhat fills a pressing need, esp for those of us with no artistic talent like me... A parametric programmer that can turn ideas and equations into graphical concepts that can then actually then be realized in the physical world, via 3D printing or other methods, is the stuff of science fiction! And now a tool that helps us write our "programs" better - just leaves me speechless (evidence to the contrary notwithstanding...) Thanks for this wonderful tool! David Merchant On 11/21/20 12:52 PM, adrianv wrote: > julianstirling wrote >> While you need to do quite a number of commands to >> accomplish tasks, there is nothing stopping you breaking these up into >> variable, smaller functions and modules. > Actually I feel like there is a trade-off here that I struggle with. If I > have a computation that I would like to embed in a function but it returns 5 > values then I have to return the five values as a list without any semantic > tags about what the list entries mean. This creates a different kind of > complexity. Do I want to have my code littered with confusing references > like return[3] and return[4]? Do I unpack the return with > > radius=return[0], > length=return[1], > scale=return[2], > etc > > Or do I just give up on modularity and embed the calculation? I tend to > find that I'm unlikely to embed a calculation unless it's actually used > multiple places because the extra mess and complexity of doing so isn't > worth it. > > > > > -- > Sent from: http://forum.openscad.org/ > > _______________________________________________ > OpenSCAD mailing list > [hidden email] > http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org _______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org |
Re: Static Code Analysis for OpenSCAD
|
|
Thanks to the wonderful example that @lar3ry and @nophead I have found a
number of issues and fixed them. SCA2D now parses NopSCADlib/libtest.scad without problems (but it takes a while!!). The only caveat to that is that list comprehensions are lexed and parsed, but not analysed yet. It took me a while to finally get the logic correct for include/use. There are a number of different ways to interpret the website when it says: > OpenSCAD executes nested calls to include and use. There is one caveat to > this, that use brings functions and modules only into the local file > context. As a result, nested calls to use have no effect on the > environment of the base file; the child use call works in the parent use > context, but the modules and functions so imported fall out of context > before they are seen by the base context. Mostly, about what happens when your chain of imports flicks back from use to import to use to import a few times. I think I have it right now! You can try the newest version with: pip install sca2d --upgrade "adrianv wrote: wrote > Actually I feel like there is a trade-off here that I struggle with. If I > have a computation that I would like to embed in a function but it returns > 5 > values then I have to return the five values as a list without any > semantic > tags about what the list entries mean. This creates a different kind of > complexity. Do I want to have my code littered with confusing references > like return[3] and return[4]? There are always trade off. The inability of C-style languages to do named variable returns does make things confusing. A static code analyser flags things that it thinks might be confusing. It is up to the author to decide what is clearest. To paraphrase Guido van Rossum - Foolishly following everything a code analyser tells you is the Hobgoblin of Little Minds. skypuppy wrote > Lexical analyzers are a BEAR to write, especially for such a niche > tool. Ha! I am starting to realise this! If I had started with these damn list comprehensions I might have just given up! Thanks for the encouragement. Does anyone know what the exact regex is that OpenSCAD uses for variable names? I have been doing this by trial and error. Perhaps because I don't understand GNU Bison. -- Sent from: http://forum.openscad.org/ _______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org |
|
|
On 22.11.20 16:30, julianstirling wrote:
> Does anyone know what the exact regex is that OpenSCAD uses > for variable names? I have been doing this by trial and error. > Perhaps because I don't understand GNU Bison. Possible names are defined in the lexer file (TOK_ID): https://github.com/openscad/openscad/blob/master/src/lexer.l#L256 ciao, Torsten. _______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
-- Torsten
|
|
I ran the latest version on my library test file, that invokes nearly all my library and got this. sca2d libtest.scad libtest.scad passed all checks! Does that mean it finds nothing controversial in my entire library or is it not testing used modules? I.e. all the tests are fairly simple code but they call into the rest of the library. On Sun, 22 Nov 2020 at 17:36, Torsten Paul <[hidden email]> wrote: On 22.11.20 16:30, julianstirling wrote: _______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org |
Re: Static Code Analysis for OpenSCAD
|
|
In reply to this post by tp3
Great. If I can ask a silly question? If the match is:
"$"?[a-zA-Z0-9_]+ what stops you naming a variable 99? Is it only priority of it deciding that is a number? What about 2() as a function? Both seem disallowed for obvious reasons. I have handled this explicitly, but I think I come to the same result in the end. -- Sent from: http://forum.openscad.org/ _______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org |
|
|
On 22.11.20 19:00, julianstirling wrote:
> Great. If I can ask a silly question? If the match is: > "$"?[a-zA-Z0-9_]+ > what stops you naming a variable 99? Is it only priority of > it deciding that is a number? What about 2() as a function? Main rule is to return the longest match, tie breaker is the rule listed first in the lexer file: https://www.cs.virginia.edu/~cr4bd/flex-manual/Matching.html#Matching I believe the TOK_ID rule is an accident (that names can start with a digit), but probably not possible to change anymore. ciao, Torsten. _______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
-- Torsten
|
Re: Static Code Analysis for OpenSCAD
|
|
In reply to this post by nophead
@nophead: "Does that mean it finds nothing controversial in my entire library
or is it not testing used modules" No. so I am using the philosophy that if you ask it to check a file then it checks just that file. It just needs to parse everything else to understand it. I am not even testing included files. I need to think about whether this is a good idea. My main thought is if I include someone else's library I can't change the code so getting tonnes of warnings is not helpful. Currently I am using bash to loop through different files in a project. This uses a lot of overhead as you reparse the files. In the next version I will make a way to loop over all files without reparsing them. -- Sent from: http://forum.openscad.org/ _______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org |
|
|
I get this running on the main file of a large project. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. WARNING:root:Unexpected args sent to warning E3002. This is a problem with SCA2D not with your scad code. Traceback (most recent call last): File "c:\users\chrisp\appdata\local\programs\python\python38-32\lib\runpy.py", line 192, in _run_module_as_main return _run_code(code, main_globals, None, File "c:\users\chrisp\appdata\local\programs\python\python38-32\lib\runpy.py", line 85, in _run_code exec(code, run_globals) File "C:\Users\ChrisP\AppData\Local\Programs\Python\Python38-32\Scripts\sca2d.exe\__main__.py", line 7, in <module> File "c:\users\chrisp\appdata\local\programs\python\python38-32\lib\site-packages\sca2d\__main__.py", line 37, in main parsed = analyser.analyse_file(args.filename, File "c:\users\chrisp\appdata\local\programs\python\python38-32\lib\site-packages\sca2d\sca2d.py", line 83, in analyse_file scope.analyse_tree(self) File "c:\users\chrisp\appdata\local\programs\python\python38-32\lib\site-packages\sca2d\outerscope.py", line 71, in analyse_tree self._check_defintions(analyser) File "c:\users\chrisp\appdata\local\programs\python\python38-32\lib\site-packages\sca2d\outerscope.py", line 81, in _check_defintions defs_and_msgs = self.get_external_defintions(analyser,count_own=False) File "c:\users\chrisp\appdata\local\programs\python\python38-32\lib\site-packages\sca2d\outerscope.py", line 124, in get_external_defintions defs = _loop_over_external_files(self._included_files, File "c:\users\chrisp\appdata\local\programs\python\python38-32\lib\site-packages\sca2d\outerscope.py", line 197, in _loop_over_external_files defs_and_msgs = file_scope.get_external_defintions(analyser, File "c:\users\chrisp\appdata\local\programs\python\python38-32\lib\site-packages\sca2d\outerscope.py", line 116, in get_external_defintions defs = _loop_over_external_files(self._used_files, File "c:\users\chrisp\appdata\local\programs\python\python38-32\lib\site-packages\sca2d\outerscope.py", line 197, in _loop_over_external_files defs_and_msgs = file_scope.get_external_defintions(analyser, File "c:\users\chrisp\appdata\local\programs\python\python38-32\lib\site-packages\sca2d\outerscope.py", line 124, in get_external_defintions defs = _loop_over_external_files(self._included_files, File "c:\users\chrisp\appdata\local\programs\python\python38-32\lib\site-packages\sca2d\outerscope.py", line 200, in _loop_over_external_files for message in defs_and_msgs[3]: TypeError: 'Message' object is not iterable On Sun, 22 Nov 2020 at 20:11, julianstirling <[hidden email]> wrote: @nophead: "Does that mean it finds nothing controversial in my entire library _______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org |
Re: Static Code Analysis for OpenSCAD
|
|
Ah man. I need to do some unit tests on the file importing! Seems to be the
biggest issue so far! So while that message is pretty unhelpful what it is saying is that it can't open the file because it cannot find the file. Are you using a library path somehow rather than a relative link? I need to get an extensive list of where OpenSCAD looks and also make a way to pass in the library file. I'll patch this bug tomorrow. -- Sent from: http://forum.openscad.org/ _______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org |
|
|
The includes / uses in the main file look like this: include <global.scad> include <positions.scad> use <extruder.scad> use <grabber.scad> use <NopSCADlib/printed/door_hinge.scad> include <door.scad> include <sides.scad> include <back.scad> include <base.scad> include <top.scad> include <front.scad> include <shelf.scad> include <NopSCADlib/printed/box_assembly.scad> The directory that contains NopSCADlib is found via the path list in OPENSCADPATH. OPENSCADPATH=W:/CHRIS/Projects/RepRaptor/OpenSCADLibs On Sun, 22 Nov 2020 at 21:20, julianstirling <[hidden email]> wrote: Ah man. I need to do some unit tests on the file importing! Seems to be the _______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org |
|
|
There seems to be a problem with for loops with more than one variable. E.g. for(i = [0 : 1], offset = i * layer_height) Gives: box.scad:169:25: W2007: Variable `i` defined but never used. box.scad:169:47: E2001: Variable `i` used but never defined. On Sun, 22 Nov 2020 at 21:38, nop head <[hidden email]> wrote:
_______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org |
|
|
I get lots of things like this cube([box_width(type) - 2 * (foot_length - foot_extension), td + 1, 1], center = true); Gives me box.scad:236:30: I1001: Overly complicated argument contains 8 tokens. Not sure it makes sense to simply count tokens. When the argument is a list the number of tokens is proportional to the list length, should it count as three arguments? And is the argument to a function call in an argument valid to be counted? On Sun, 22 Nov 2020 at 22:22, nop head <[hidden email]> wrote:
_______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org |
Re: Static Code Analysis for OpenSCAD
|
|
In reply to this post by tp3
On 11/22/2020 11:58 AM, Torsten Paul
wrote:
I believe the TOK_ID rule is an accident (that names can start with a digit), but probably not possible to change anymore. I was kind of surprised to find that I could name a module "2x4", but it was nice for modeling lumber. _______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org |
|
|
On Mon, Nov 23, 2020 at 07:56:10AM +0000, Jordan Brown wrote:
> On 11/22/2020 11:58 AM, Torsten Paul wrote: > > I believe the TOK_ID rule is an accident (that names can start with a > > digit), but probably not possible to change anymore. > > I was kind of surprised to find that I could name a module "2x4", but it > was nice for modeling lumber. Aaargh! But then 1a2, 1b3, 1c4 and 1d5 are valid as a variable name, but 1e6 is not? Sigh! Roger. -- ** [hidden email] ** https://www.BitWizard.nl/ ** +31-15-2049110 ** ** Delftechpark 11 2628 XJ Delft, The Netherlands. KVK: 27239233 ** f equals m times a. When your f is steady, and your m is going down your a is going up. -- Chris Hadfield about flying up the space shuttle. _______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org |
|
|
On 23.11.20 09:23, Rogier Wolff wrote:
> Aaargh! But then 1a2, 1b3, 1c4 and 1d5 are valid as a variable name, > but 1e6 is not? Sigh! Correct. But then, there's the option to just not use those. ciao, Torsten. _______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
-- Torsten
|
Re: Static Code Analysis for OpenSCAD
|
|
"Correct. But then, there's the option to just not use those. " - I can make
the linter shouty if you use them :D. I plan eventually to have a configurable regex for nice variable names/module names/function names. @nophead I did not push out the promised fix today. Busy busy busy. Still working at past midnight on something else. Thanks for the double loop bug. Yeah I treat them the same as I would key word arguments. I can fix that. Ah I do not check "OPENSCADPATH". I shall look into this. Will probably be weird to get it platform independent, but I am on linux you are on windows so I may have to carry on using you as an alpha tester. Totally agree about the token counting being dumb. It is in there for my own purposes as I am cleaning up messy code and so it gives me a sniff of where to look. But yeah, need to think about complexity in a more intelligent way. Especially as any decent size list gets flagged! I was thinking of adding the complexity largest two elements in the list. This way it represents a list being more complex than a single token, but doesn't scale stupidly. -- Sent from: http://forum.openscad.org/ _______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org |
|
|
It is easy to handle OPENSCADPATH in Python, including the platform difference: os.environ['OPENSCADPATH'].split(os.pathsep) Gives you a list of paths to look in for libraries. os.pathsep is ';' for Windows and ':' for others. To remove some of the warnings about argument tokens I factored out some common sub-expressions into variables. So perhaps there should be a warning about repeated subexpressions. On Tue, 24 Nov 2020 at 00:40, julianstirling <[hidden email]> wrote: "Correct. But then, there's the option to just not use those. " - I can make _______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org |
|
|
In reply to this post by doug.moen
Ahem, where is that thread in the forum?
_______________________________________________ OpenSCAD mailing list [hidden email] http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
-- Torsten
|
«
Return to OpenSCAD
|
1 view|%1 views
| Free forum by Nabble | Edit this page |