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

Ruby parse errors #2291

Closed
4 tasks done
mschwager opened this issue Dec 18, 2020 · 7 comments
Closed
4 tasks done

Ruby parse errors #2291

mschwager opened this issue Dec 18, 2020 · 7 comments
Assignees
Labels
bug Something isn't working good first task Would take a bit longer to fix than "good first issue" lang:ruby tree-sitter user:internal requested only by someone within Semgrep Inc.

Comments

@mschwager
Copy link
Contributor

mschwager commented Dec 18, 2020

Closing out #2231, so I'm creating this issue to track additional known Ruby parsing bugs.

Once these are fixed upstream we can pull in the changes and close this out 👍

@mschwager mschwager added bug Something isn't working tree-sitter lang:ruby labels Dec 18, 2020
@aryx aryx added the good first task Would take a bit longer to fix than "good first issue" label Jan 8, 2021
@DrewDennison DrewDennison added the user:internal requested only by someone within Semgrep Inc. label Jan 13, 2021
@nbrahms nbrahms self-assigned this Jan 20, 2021
@nbrahms
Copy link
Contributor

nbrahms commented Jan 20, 2021

cf. tree-sitter/tree-sitter-ruby#153

@nbrahms
Copy link
Contributor

nbrahms commented Jan 20, 2021

cf. tree-sitter/tree-sitter-ruby#154

@nbrahms
Copy link
Contributor

nbrahms commented Jan 20, 2021

@mschwager @minusworld Should tree-sitter/tree-sitter-ruby#155 affect our GA?

@mschwager
Copy link
Contributor Author

@mschwager @minusworld Should tree-sitter/tree-sitter-ruby#155 affect our GA?

Ruby support is already GA'ed. Or do you mean demote Ruby maturity? Considering this bug doesn't violate any of our maturity definitions, I'd vote no. IMO we can treat it like a parsing bug in any other GA language.

@nbrahms
Copy link
Contributor

nbrahms commented Jan 20, 2021

@mschwager @minusworld Should tree-sitter/tree-sitter-ruby#155 affect our GA?

Ruby support is already GA'ed. Or do you mean demote Ruby maturity? Considering this bug doesn't violate any of our maturity definitions, I'd vote no. IMO we can treat it like a parsing bug in any other GA language.

This won't cause parse errors. Instead, it will cause certain patterns not to match code that you might expect them to.

@mschwager
Copy link
Contributor Author

Hmm, your question is telling me that you think we should. Could you elaborate on your thoughts?

I would still lean toward "no" for the following reasons:

  • We haven't had any complaints from external users, or internal users, for that matter, we found it through manual code review/debugging. This makes me think that it's not a widespread issue.
  • IMO, we should prefer to fix bugs in GA languages vs. demoting them. Demoting feels like moving backward instead of forward.
  • Again, this doesn't violate any of our maturity definitions. We can always update the definitions, but I'm having a tough time thinking of a generalized, quantifiable rule for this. I.e. how is this different than a false negative bug in any of our other GA languages? Ensuring no false negatives is a tall order :)

What do you think?

@aryx
Copy link
Collaborator

aryx commented Feb 11, 2021

I think most of the things have been merged with #2531, and ruby is at 99.944% I think.

@aryx aryx closed this as completed Feb 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first task Would take a bit longer to fix than "good first issue" lang:ruby tree-sitter user:internal requested only by someone within Semgrep Inc.
Projects
None yet
Development

No branches or pull requests

4 participants