Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Aug 8, 2018

This PR Fixes ODASA-7268 by flagging constructors with invalid this/super expressions instead of the invalid expressions themselves. The expressions are still a part of the error message.

This will prevent artificially high alert counts for some (legacy?) constructors.
Example: https://lgtm.com/projects/g/angular/watchtower.js/snapshot/50cb876f1b62f16e9beea51c8ca4209eadf84876/files/src/watch_group.js?sort=name&dir=ASC&mode=heatmap&showExcluded=true#x76858207ab8a9450:1

sc.getEnclosingFunction() != ctor
)
select e, "The super constructor must be called before using '" + kind + "'."
select ctor, "The super constructor must be called before using '$@'.", e, kind
Copy link

Choose a reason for hiding this comment

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

We generally want to avoid flagging multi-line entities, otherwise everything ends up looking very red on LGTM. You can use the RestrictedLocations library to only highlight the first line of the constructor.

Copy link
Author

Choose a reason for hiding this comment

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

Done. Good catch.

@xiemaisi xiemaisi added the JS label Aug 8, 2018
@xiemaisi xiemaisi merged commit e32dc08 into github:master Aug 9, 2018
aibaars pushed a commit that referenced this pull request Oct 14, 2021
smowton pushed a commit to smowton/codeql that referenced this pull request Oct 28, 2021
Kotlin: Improve the dbscheme generator
erik-krogh pushed a commit to erik-krogh/ql that referenced this pull request Dec 15, 2021
Autogenerate QLDoc for `TreeSitter.qll`
erik-krogh pushed a commit to erik-krogh/ql that referenced this pull request Dec 15, 2021
Autogenerate QLDoc for `TreeSitter.qll`
dbartol pushed a commit that referenced this pull request Dec 18, 2024
dbartol pushed a commit that referenced this pull request Dec 18, 2024
Treat branch-deploy action as a source of HEAD ref for untrusted checkouts
MathiasVP pushed a commit to MathiasVP/ql that referenced this pull request Aug 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant