[BUG] Invalid "Missing NEXT statement"

[mikes-zum]

Describe the bug Missing next statement error generated on this line of code:

FOR I = 32 TO 127; OUT I; NEXT I ;* Printable chars

To Reproduce Insert this line of code in VS Code

See error and "squiggle" underline

Expected behavior Code should be accepted Screenshots

Versions of applicable software Windows 10 VS Code Insiders 1.42.0-insider MV Basic 2.0.2

Code samples and/or reproduceable test cases

FOR I = 32 TO 127; OUT I; NEXT I ;* Printable chars

Additional context

[mikes-zum]

[mikes-zum]

I should also mention:

},
            "MVBasic.languageType": {
                "scope": "resource",
                "type": "string",
                "default": "jBASE",
                "enum": [
                    "MVON",
                    "jBASE",
                    "UniVerse",
                    "OpenQM"
                ],

[kpowick]

It should probably just be a warning that, though valid, this code is poorly written. ;)

[mikes-zum]

I am in the process of validating all of my jBASE documentation changes and in most cases also using the code examples from the same pages as the documentation.

The resulting source code may then be useful for (MVBasic) testing purposes .

[ianmcgowan]

This is caused by validateTextDocument in server/src/server.ts. This function is called whenever the document changes, and goes through line-by-line validating (as the name suggests :-)

For FOR/NEXT loops, it builds a cross-reference as it goes through the program, also incrementing a counter for each FOR and another for each NEXT. If the counts don't match at the end, it goes through the cross-reference crossing out the ones that do match and any missing ones are added to the list of diagnostic warnings.

E.g. "1~+F|3~-F" if the program has FOR F=1 TO 10 in line 1, and then NEXT F in line 3.

One fix is to have some slightly more complex parsing for the FOR F=1 TO 10; CRT F; NEXT F case, where the trailing NEXT F is picked up and handled as though it was on its own line.

Another fix, arguably the correct one - the parser should break statements with embedded semi-colons onto their own lines and then process them independently (like the BASIC compiler would). This is technically correct, but carries a lot more risk of getting things wrong.

[ianmcgowan]

I created a pull request https://github.com/mvextensions/mvbasic/pull/63 for a change to fix this - it works by splitting lines with embedded semi-colons into separate new lines. That should help any linting errors with multiple statements on one line, but does have the unfortunate side-effect of throwing off the line number reported for the error. Also improved the FOR/NEXT checking (I think), but would like Grant's opinion on the change.

[kpowick]

@ianmcgowan Does this only apply to FOR/NEXT checking? i.e. The file is parsed in this new way only to calculate FOR/NEXT?

If so, does semicolon checking consider the implied "END" statement in single-line, multi-statement IF/THEN constructs? Example:

IF TRUE THEN CRT "TRUE";STOP

The compiler interprets the above as:

IF TRUE THEN
   CRT "TRUE"
   STOP
END

I guess I would have a better idea of how this worked if I looked at the project code, but I haven't had time. It's just that this issue caught my attention.

[ianmcgowan]

Shoot, @kpowick , that's a really good point. No, this change is for all lines with substatements. The linter (w/this pull request), would treat that example as:

IF TRUE THEN CRT "TRUE"
STOP

Which is ok from a linting perspective, but if there're ELSE's and END's in there may fall apart.

I probably need to add two test R83 basic programs: one with legal syntax but including as many tricky cases as possible, and another with tons of illegal syntax to make sure the linter is catching it all.

It's possible to figure it out (the basic compiler does it after all), just not sure how much javascript code that would end up taking. I'm thinking a lot :-(

[itsxallwater]

I like the idea of adding additional test programs. You'll see we've started to do that under the tests directory, specifically for jBASE. By all means push a PR to add more! Will make all of this dev much easier, and at some point I'm hopefully to setup some automated testing that'll leverage them too.

[kpowick]

...That should help any linting errors with multiple statements on one line, but does have the unfortunate side-effect of throwing off the line number reported for the error.

Does the line number being thrown off affect only the specific For/Next error line, or does it throw off all subsequent error line reporting? Just wondering how "ugly" it could get.

[ianmcgowan]

It affects all subsequent lines, but I'm working on something to store the original line number, and report that in the error.

https://github.com/ianmcgowan/mvbasic/blob/for_next_pull_2/tests/pick/R83GOOD.b

Is an initial test file (of things that are legal syntax), and it does seem that linting is not thrown off by splitting lines, at least with my basic BASIC syntax.

On Mon, Dec 23, 2019 at 11:18 AM Kevin Powick notifications@github.com wrote:

...That should help any linting errors with multiple statements on one line, but does have the unfortunate side-effect of throwing off the line number reported for the error.

Does the line number being thrown off affect only the specific For/Next error line, or does it throw off all subsequent error line reporting? Just wondering how "ugly" it could get.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mvextensions/mvbasic/issues/62?email_source=notifications&email_token=AADD3D37TGB3QUHWENVCAYTQ2EFKBA5CNFSM4J5LMC3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHRYJ2Y#issuecomment-568558827, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADD3D2SPHV75J3Q6PAUJRLQ2EFKBANCNFSM4J5LMC3A .

[itsxallwater]

This may be resolved now between latest + https://github.com/mvextensions/mvbasic/pull/94. Revisited some of the above samples and I'm no longer seeing any issues. @kpowick want to put eyes on this too?