Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Not enough whitespace after ... mark is recognized as new variable #460

Closed
bhirsz opened this issue Sep 3, 2021 · 9 comments · Fixed by #483
Closed

[Bug] Not enough whitespace after ... mark is recognized as new variable #460

bhirsz opened this issue Sep 3, 2021 · 9 comments · Fixed by #483
Labels
bug Something isn't working
Milestone

Comments

@bhirsz
Copy link
Member

bhirsz commented Sep 3, 2021

This code:

${VARIABLE}=
... """                                            ${\n}
... data:                             ${\n}
... ( a b c ) = ( 1  2  3 ) ${\n}
... """

produces:

test.robot:23:0 [E] 0803 Multiple variables with name "... """" in Variables section. Note that Robot Framework is case-insensitive (duplicated-variable)
test.robot:26:0 [E] 0803 Multiple variables with name "... """" in Variables section. Note that Robot Framework is case-insensitive (duplicated-variable)

I think it should rather be catched by "not-enough-whitespace-after-newline-marker" rule (but it's keyword naming rule.. maybe we should export it to be general rule checking all statements that starts from the beginning of line and contains ...?)

@bhirsz bhirsz added the bug Something isn't working label Sep 3, 2021
@mnojek
Copy link
Member

mnojek commented Sep 3, 2021

Yeah, I think this rule should not be reported. The cause here is actually what you wrote ("not-enough-whitespace-after-newline-marker") so I tihnk this should be reported. Also, it would be nice to have this as a Token, to be able to catch it easier. Do you remember which token is it (this new line marker)?

@bhirsz
Copy link
Member Author

bhirsz commented Sep 3, 2021

Token.CONTINUATION but in this case if we don't provide enough spaces it will not be recognizes as such. In provided example Robot Framework thinks it's variable name (but for some reason it doesn't complain with syntax error for variable with name ...).

@bhirsz
Copy link
Member Author

bhirsz commented Sep 3, 2021

So there are two tasks a) make duplicated-variable immune to ... not enough whitespace b) export not-enough-whitespace-after-newline-marker to spacing rules and match more cases (keywords, variables, imports, settings).

@mnojek
Copy link
Member

mnojek commented Sep 3, 2021

Token.CONTINUATION but in this case if we don't provide enough spaces it will not be recognizes as such. In provided example Robot Framework thinks it's variable name (but for some reason it doesn't complain with syntax error for variable with name ...).

What about involving @pekkaklarck into it? Maybe something on the parsing side can be improved about it?

@bhirsz
Copy link
Member Author

bhirsz commented Sep 3, 2021

I've noticed it's parsed and catched:
image
It's just that for some reason we're not parsing this error - I will open separate bug report.

@bhirsz
Copy link
Member Author

bhirsz commented Sep 3, 2021

Also (space before ...) -> I it's also reported as not left aligned variable (which is fine):

Multiple variables with name " ..." in Variables section. Note that Robot Framework is case-insensitive (duplicated-variable)

@pekkaklarck
Copy link

Parsing already detects the error. It notices that ... whatever isn't a valid variable name and reports that. It's just doesn't generate error token or node in this situation, instead the variable node has errors attribute containing the error.

Error handling in parsing could be enhanced to handle this particular case a bit better, though. We could report a slightly different error if the name starts with ... . Possibly we could just add something like

Did you forget to add enough spaces after ...?

after the current message.

@bhirsz
Copy link
Member Author

bhirsz commented Sep 3, 2021

Parsing already detects the error. It notices that ... whatever isn't a valid variable name and reports that. It's just doesn't generate error token or node in this situation, instead the variable node has errors attribute containing the error.

Error handling in parsing could be enhanced to handle this particular case a bit better, though. We could report a slightly different error if the name starts with ... . Possibly we could just add something like

Did you forget to add enough spaces after ...?

after the current message.

It doesnt matter for us since we will add extra parsing and deal with reporting the issue but it would be certainly good addition for RF users if they encounter such problem.

@pekkaklarck
Copy link

Feel free to submit issues, and PRs, about error messages that can be enhanced!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants