Skip to content

Warning on comment references for Dart keywords #60794

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

Open
FMorschel opened this issue May 26, 2025 · 7 comments
Open

Warning on comment references for Dart keywords #60794

FMorschel opened this issue May 26, 2025 · 7 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-warning Issues with the analyzer's Warning codes P3 A lower priority bug or feature request

Comments

@FMorschel
Copy link
Contributor

From @bwilkerson's #47444 (comment):

There is no explicit declaration of this, so it isn't clear where the user would expect that to link to.

From @srawlins' #47444 (comment):

[W]e made an explicit decision to stop supporting [this] in dartdoc, as analyzer does not support it, and it makes sense to not support it.

Maybe we could think of other cases like this, but at least for this, super, false, true, null, etc, we could probably create a warning to suggest the user to stop using [] and to prefer backsticks.

@FMorschel FMorschel added the area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. label May 26, 2025
@bwilkerson
Copy link
Member

If dartdoc isn't going to support these then it makes sense for us to let users know. We already have INVALID_COMMENT_REFERENCE to flag other kinds of invalid references. We should think about reusing this same warning (possibly with a different code if the message should be different) to report these cases.

@bwilkerson bwilkerson added P3 A lower priority bug or feature request devexp-warning Issues with the analyzer's Warning codes labels May 27, 2025
@srawlins
Copy link
Member

Maybe we could think of other cases like this, but at least for this, super, false, true, null, etc, we could probably create a warning to suggest the user to stop using [] and to prefer backticks.

I think these are all reported by the comment_references lint rule. See the tests: https://github.com/dart-lang/sdk/blob/main/pkg/linter/test/rules/comment_references_test.dart#L249

@FMorschel
Copy link
Contributor Author

FMorschel commented May 27, 2025

They are reported as if the keywords don't exist in scope. I'd prefer the INVALID_COMMENT_REFERENCE warning. They could only be triggered when that lint is on, but for keywords, I don't see any problem in having that all the time. Of course, usable (we can name things with them) keywords like async and such, should not trigger.

Image

@srawlins
Copy link
Member

Ah I see. I think we can just add an additional LintCode then to the comment_references AnalysisRule. The LintCode is what controls the messaging.

@bwilkerson
Copy link
Member

I think these are all reported by the comment_references lint rule.

Interesting. It appears that almost nobody has that lint enabled. I don't know whether that's because nobody cares about this or whether nobody knows the lint exists. If it's the former then we don't really need to do anything about this. If it's the latter then we should consider whether a warning would be more appropriate.

@srawlins
Copy link
Member

I'd love to convert it to a warning. But I'm pretty sure there will always be false positives until we can use a real markdown parser in the rule. I don't have any examples offhand. But a real markdown parser can handle escaped characters like [foo\]. There are a lot of weird corner cases when parsing markdown links. And then tracking inline code, with backticks. And then escaped backticks...

@FMorschel
Copy link
Contributor Author

I only got the knowledge of this lint recently, so I suspect the latter. I would even argue that it should probably be on effective Dart now that we have doc imports, but I guess we could wait until the fixes produce the right import kind at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-warning Issues with the analyzer's Warning codes P3 A lower priority bug or feature request
Projects
None yet
Development

No branches or pull requests

3 participants