Skip to content

JS: only set the file in the diagnostics message if the file is within the source root #12742

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

Closed
wants to merge 1,136 commits into from

Conversation

erik-krogh
Copy link
Contributor

@erik-krogh erik-krogh commented Apr 3, 2023

This should fix #12734

I wasn't able to replicate locally, but the stack-trace in the issue provided enough information.

It was this change that caused the null-pointer: #12113 (comment)

d10c and others added 30 commits March 27, 2023 23:01
Optional content flow through constructors remains.
Again, this is hacky; we don't distinguish rigorously between an
optional value and its content (similar to how it was before enum
content flow).
…eorder

Ruby: Order synthetic children in PrintAST based on their index instead of location
…direct-type-inference

python: Fix link to type inference
…-binary

C#: Improve cs/web/debug-binary to repect the RemoveAttributes transformation.
…l-namespace

RB: always resolve toplevel namespaces to their locally qualified name
…for-javascript.rst

Co-authored-by: Andrew Eisenberg <aeisenberg@github.com>
…in-use`

Notice that it doesn't find the potentially unsafe version, or the vuln that spans calls.
Copy link
Contributor

@aibaars aibaars left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@aibaars aibaars left a comment

Choose a reason for hiding this comment

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

Perhaps this should be for the rc/3.9 branch

if (file.startsWith(LGTM_SRC)) {
relativeFilePath = file.subpath(LGTM_SRC.getNameCount(), file.getNameCount()).toString();
builder = builder.setFile(file.subpath(LGTM_SRC.getNameCount(), file.getNameCount()).toString());
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we skip diagnostics, or perhaps just diagnostic locations, for files outside the source root? I'm not sure how useful it is to have a location with a range but without a file path.

@erik-krogh erik-krogh changed the base branch from main to codeql-cli-2.12.6 April 3, 2023 10:36
@erik-krogh erik-krogh requested review from a team as code owners April 3, 2023 10:36
@erik-krogh erik-krogh closed this Apr 3, 2023
@erik-krogh
Copy link
Contributor Author

Sorry for the ping, bad change of branch.
I'll reopen another PR.

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.

[Vue.js + TS] Perform CodeQL Analysis fails when tuple is used as a type in a vue component