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

Upgrade tree-sitter to the latest version #233

Merged
merged 3 commits into from
Feb 27, 2024
Merged

Conversation

juli1
Copy link
Collaborator

@juli1 juli1 commented Feb 26, 2024

What problem are you trying to solve?

We are using an old version of tree-sitter. We should upgrade to the latest.

What is your solution?

Upgrade tree-sitter.

Alternatives considered

Do not upgrade

What the reviewer should know

To upgrade Swift, we had to download the generated parser from their GitHub action and publish them on a repository. This is because the tree-sitter-swift maintainers do not want to publish the parser.c and scanner.c. We opened a bug report to ask them to do so but in the meantime, we have a workaround.

Testing

  • large repo python: found one issue with python-code-style/function-naming. Fixed according to the new predicates reported here
  • large repo JS/TS: more violations caught in our analyzer. After investigations, this is related to only one very long file. I suspect the issue is more related to parsing and bugfix than anything else.
  • large repo Java: done, no difference
  • large repo go: in no difference

Testing Methodology

  1. Run the analyzer on previous and new version
datadog-static-analyzer  --directory path/to/repo --output result-[old|new].csv --format csv

If the same number of violations is found, all good. Otherwise, keep digging.

  1. Sort the results
sort result-old.csv > results-old-sorted.csv
  1. Check the difference
comm -3 results-old-sorted.csv results-new-sorted.csv

@juli1 juli1 requested a review from a team as a code owner February 26, 2024 21:46
Cargo.toml Show resolved Hide resolved
@alex-pinkus
Copy link

Rather than hosting the artifacts yourself , you might as well use the with-generated-files branch I already publish: https:/alex-pinkus/tree-sitter-swift/tree/with-generated-files

Of course, as I mentioned on the linked issue, you shouldn’t necessarily treat that as a long-term viable approach.

@juli1
Copy link
Collaborator Author

juli1 commented Feb 27, 2024

Rather than hosting the artifacts yourself , you might as well use the with-generated-files branch I already publish: https:/alex-pinkus/tree-sitter-swift/tree/with-generated-files

Of course, as I mentioned on the linked issue, you shouldn’t necessarily treat that as a long-term viable approach.

Thank you so much. I used the branch you provided, it's way cleaner this way. Thank you again!

@juli1
Copy link
Collaborator Author

juli1 commented Feb 27, 2024

I tested this new version against large repositories and there is an issue with the new tree-sitter update (e.g. this new version we try to use). One rule (python-code-style/function-naming) is not working correctly and it seems there is a regression in tree-sitter.

I opened a bug for tree-sitter here. I will keep this pull request open until we come to a conclusion with the tree-sitter maintainers (either we need to fix something or they need to fix something).

@juli1 juli1 merged commit a00c22e into main Feb 27, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants