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

Move has_comments to CommentRanges #11495

Merged
merged 1 commit into from
May 22, 2024
Merged

Move has_comments to CommentRanges #11495

merged 1 commit into from
May 22, 2024

Conversation

dhruvmanila
Copy link
Member

Summary

This PR moves the has_comments function from Indexer to CommentRanges. The main motivation is that the CommentRanges will now be built by the parser which is shared between the linter and the formatter. Thus, the CommentRanges will be removed from the Indexer.

Test Plan

cargo test

@dhruvmanila dhruvmanila added the internal An internal refactor or improvement label May 22, 2024
@dhruvmanila dhruvmanila enabled auto-merge (squash) May 22, 2024 13:31
@dhruvmanila dhruvmanila merged commit f0046ab into main May 22, 2024
17 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/has-comments branch May 22, 2024 13:35
Copy link
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@@ -384,7 +384,11 @@ pub(crate) fn unittest_raises_assertion(
},
call.func.range(),
);
if !checker.indexer().has_comments(call, checker.locator()) {
if !checker
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: You could have kept the has_comments method on Indexer and simply forwarded the call to CommentRanges to reduce the diff size (It also increases encapsulation because callers don't need to know how the Indexer represents the data internally)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if that makes any difference in terms of encapsulation because CommentRanges is already exposed via indexer.comment_ranges and calls to the other methods on CommentRanges are done directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal An internal refactor or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants