mozilla / rust-code-analysis

Library to analyze and collect metrics on source code
https://mozilla.github.io/rust-code-analysis/
289 stars 49 forks source link

Fix test failures introduced by #1086 #1091

Open marco-c opened 3 months ago

marco-c commented 3 months ago

@alexle0nte are you going to work on this?

alexle0nte commented 3 months ago

Yes, I'll create a PR to fix all the tests

marco-c commented 2 months ago

@alexle0nte are you making progress on this?

alexle0nte commented 2 months ago

@marco-c Yes, #1125 fixes all the metrics tests. However, there is a problem with repository tests, whose snapshots are contained in rca-output.

Version 0.21.2 of tree-sitter-rust has a bug regarding parsing doc comments, that causes many serde tests to fail. This bug has been fixed in version 0.23.0, however, upgrading tree-sitter-rust to this version would also require upgrading all other grammars to 0.23.0, but tree-sitter-kotlin doesn't yet support tree-sitter 0.23.0.

In the meantime, I could update the snapshots of all the files that don't contain doc comments, and then, when version 0.23.0of tree-sitter-kotlin is released, we can do a new grammar upgrade and update the remaining snapshots as well.

marco-c commented 2 months ago

@alexle0nte do you think we could submit a PR to tree-sitter-kotlin to support 0.23.0 or is it hard?

Your plan seems fine otherwise.

alexle0nte commented 2 months ago

@marco-c I've just opened a PR in tree-sitter-kotlin to bump the tree-sitter dependency to 23.0. Once it's merged and the new release is out, we can upgrade all the grammars to 23.0.

marco-c commented 2 months ago

Thanks! If it still open in a couple of week, we could create a temporary fork to unblock the updates.

Luni-4 commented 2 months ago

@marco-c and @alexle0nte

We can also ask tree-sitter developers if they are willing to adopt the Kotlin grammar among their organization's parsers

marco-c commented 4 weeks ago

@Luni-4 @alexle0nte it looks like the PR is still not merged, wdyt we should do?

alexle0nte commented 3 weeks ago

@Luni-4 @alexle0nte it looks like the PR is still not merged, wdyt we should do?

I think we can create a temporary fork as you suggested and use that until it gets merged