chappertron / lammps-analyser

A linter for the LAMMPS scripting language
GNU General Public License v2.0
0 stars 0 forks source link

Failing to parse function calls within quoted expressions #11

Closed chappertron closed 1 month ago

chappertron commented 1 month ago

The following valid snippet is giving errors

variable n_bins equal "floor(lz/v_bw_target)" 

The parser seems to be hallucinating variables from an error node,

../in.nemd|40 col 24-25 error| 40:24: Undefined variable `f`
../in.nemd|40 col 24-29 error| Invalid Syntax `floor`
../in.nemd|40 col 25-26 error| 40:25: Undefined variable `l`
../in.nemd|40 col 26-27 error| 40:26: Undefined variable `o`
../in.nemd|40 col 27-28 error| 40:27: Undefined variable `o`
../in.nemd|40 col 28-29 error| 40:28: Undefined variable `r`

There are two issues here:

For completeness, the tree that is being parsed is:

  (command ; [39, 0] - [39, 45]
    (variable_def ; [39, 0] - [39, 45]
      (variable) ; [39, 9] - [39, 15]
      style: (variable_style) ; [39, 16] - [39, 21]
      (quoted_expression ; [39, 22] - [39, 45]
        (ERROR ; [39, 23] - [39, 28]
          (variable) ; [39, 23] - [39, 24]
          (variable) ; [39, 24] - [39, 25]
          (variable) ; [39, 25] - [39, 26]
          (variable) ; [39, 26] - [39, 27]
          (variable)) ; [39, 27] - [39, 28]
        (expression ; [39, 28] - [39, 44]
          (parens ; [39, 28] - [39, 44]
            (expression ; [39, 29] - [39, 43]
              (binary_op ; [39, 29] - [39, 43]
                left: (expression ; [39, 29] - [39, 31]
                  (thermo_kwarg)) ; [39, 29] - [39, 31]
                right: (expression ; [39, 32] - [39, 43]
                  (underscore_ident ; [39, 32] - [39, 43]
                    (variable)))))))))) ; [39, 34] - [39, 43]

As you can see each letter of the function call is being parsed as a seperate variable

chappertron commented 1 month ago

The issue in the grammar turned out to be due to the floor function being missing from the list of function names. This really to accept any function name.

The issue with parsing error nodes in lammps-analyser needs to be handled, however.

chappertron commented 1 month ago

This has been fixed. I can't remember when, however.