Skip to content

Erroneous location mapping #967

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
zbazztian opened this issue Oct 7, 2021 · 1 comment
Open

Erroneous location mapping #967

zbazztian opened this issue Oct 7, 2021 · 1 comment
Labels
bug Something isn't working good first issue Good for newcomers VSCode

Comments

@zbazztian
Copy link

VSCode only handles locations corresponding to a range. This is a known shortcoming. This also means special ranges can correspond to invalid VSCode ranges which mean that no scroll happens. This likely has never worked 100% but we did fix some off by one errors which may have caused it to seem to work.

Version
Extension v1.5.5
CodeQL CLI v2.6.2
VSCode:

1.60.2
7f6ab5485bbc008386c4386d08766667e155244e
x64

To reproduce
Consider the following screenshot, with a query that emits location links to a file in the database. Only a small number of these locations actually work (in the sense that they can be clicked and VSCode selects the given location).
locations

As is visible in the output (Problem View), most of the locations are mapped to the top of the file. According to our documentation, however, :2:0:2:0, for example, should mark the entire second line.

This is a somewhat serious problem, because it prevents results from being properly accessible. We recently encountered issues with our Java JSP support, where the resulting locations in the JSP files all pointed at the top of the files, making triaging the results very tedious.

Expected behavior
Location display in VSCode should be consistent with our documentation.

Additional context
Here the query that was used in the screenshot above:

/**
 * @kind problem
 */// documentation at https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/#file-urls
class MyLoc extends string {
  string pos;
  MyLoc() {
    this = "file:///home/sebastian/dev/src/commons-io/src/main/java/org/apache/commons/io/IOUtils.java" + pos and
    pos = [
      ":0:0:0:0", // (works) docs say that this should link to the whole file
      ":2:0:2:0", // (fails) docs say that this should mark the entire line
      ":2:0:2:1", // (fails) should probably not work
      ":2:0:3:0", // (fails) I would expect it to mark lines 2 and 3
      ":2:1:2:0", // (fails) not sure whether it should work
      ":2:1:3:0", // (fails) This is currently used to mark JSP results. Yorck says it used to work.
      ":2:1:2:1", // (works)
      ":2:1:3:1"  // (works)
    ]
  }
  string getURL() { result = this }
  string getPos() { result = pos }
}from MyLoc l
select l, "Position " + l.getPos() + " is mapped to "

@alexet

@zbazztian zbazztian added the bug Something isn't working label Oct 7, 2021
@github-actions github-actions bot added the VSCode label Oct 7, 2021
@aeisenberg
Copy link
Contributor

Thanks for reporting. I haven't taken a deep look at this yet, but I suspect the problem has to do with our showLocation logic:

export async function showLocation(location?: Location) {
if (!location) {
return;
}
const doc = await workspace.openTextDocument(location.uri);
const editorsWithDoc = Window.visibleTextEditors.filter(
(e) => e.document === doc
);
const editor =
editorsWithDoc.length > 0
? editorsWithDoc[0]
: await Window.showTextDocument(
doc, {
// avoid preview mode so editor is sticky and will be added to navigation and search histories.
preview: false,
viewColumn: ViewColumn.One,
});
const range = location.range;
// When highlighting the range, vscode's occurrence-match and bracket-match highlighting will
// trigger based on where we place the cursor/selection, and will compete for the user's attention.
// For reference:
// - Occurences are highlighted when the cursor is next to or inside a word or a whole word is selected.
// - Brackets are highlighted when the cursor is next to a bracket and there is an empty selection.
// - Multi-line selections explicitly highlight line-break characters, but multi-line decorators do not.
//
// For single-line ranges, select the whole range, mainly to disable bracket highlighting.
// For multi-line ranges, place the cursor at the beginning to avoid visual artifacts from selected line-breaks.
// Multi-line ranges are usually large enough to overshadow the noise from bracket highlighting.
const selectionEnd =
range.start.line === range.end.line ? range.end : range.start;
editor.selection = new Selection(range.start, selectionEnd);
editor.revealRange(range, TextEditorRevealType.InCenter);
editor.setDecorations(shownLocationDecoration, [range]);
editor.setDecorations(shownLocationLineDecoration, [range]);
}

Just marking this now so that whoever looks at this later will know where to start.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers VSCode
Projects
None yet
Development

No branches or pull requests

2 participants