-
Notifications
You must be signed in to change notification settings - Fork 1.7k
QL: improve the "this block-comment should have been a QLDoc"-query #11294
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C# LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C++ differences LGTM.
node instanceof TopLevel and | ||
comment = getCommentAtStart(node.getLocation().getFile(), 1) and | ||
nodeDescrip = "the file" | ||
) and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels like we could remove some redundant logic here by adding a reason
parameter to getACommentThatCouldBeQLDoc
returning "the below code"
or "the file"
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that could save some duplication.
But that sounds like something for a followup, this seems good enough to merge.
This would have caught a recent QLDoc check that I spent way too long trying to fix, where I didn't realise that the top-level comment was not a QLDoc.
I also fixed a whole bunch of the issues found by the query.
Not all, some results are benign.
And also some improvements to how we match up QLDoc in QL-for-QL.