Skip to content

Include column numbers in location URLs #2406

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

Merged
merged 3 commits into from
Jul 19, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions extensions/ql-vscode/src/common/bqrs-utils.ts
Original file line number Diff line number Diff line change
@@ -142,5 +142,7 @@ export function tryGetRemoteLocation(
fileLink,
resolvableLocation.startLine,
resolvableLocation.endLine,
resolvableLocation.startColumn,
resolvableLocation.endColumn,
);
}
14 changes: 13 additions & 1 deletion extensions/ql-vscode/src/common/location-link-utils.ts
Original file line number Diff line number Diff line change
@@ -4,8 +4,20 @@ export function createRemoteFileRef(
fileLink: FileLink,
startLine?: number,
endLine?: number,
startColumn?: number,
endColumn?: number,
): string {
if (startLine && endLine && startLine !== endLine) {
if (
startColumn &&
endColumn &&
startLine &&
endLine &&
// Verify that location information is valid; otherwise highlighting might be broken
((startLine === endLine && startColumn < endColumn) || startLine < endLine)
) {
// This relies on column highlighting of new code view on GitHub
return `${fileLink.fileLinkPrefix}/${fileLink.filePath}#L${startLine}C${startColumn}-L${endLine}C${endColumn}`;
} else if (startLine && endLine && startLine < endLine) {
return `${fileLink.fileLinkPrefix}/${fileLink.filePath}#L${startLine}-L${endLine}`;
} else if (startLine) {
return `${fileLink.fileLinkPrefix}/${fileLink.filePath}#L${startLine}`;
Original file line number Diff line number Diff line change
@@ -148,8 +148,7 @@ function generateMarkdownForInterpretedResult(
lines.push(
createMarkdownRemoteFileRef(
interpretedResult.fileLink,
interpretedResult.highlightedRegion?.startLine,
interpretedResult.highlightedRegion?.endLine,
interpretedResult.highlightedRegion,
),
);
lines.push("");
@@ -250,8 +249,7 @@ function generateMarkdownForAlertMessage(
} else if (token.t === "location") {
alertMessage += createMarkdownRemoteFileRef(
token.location.fileLink,
token.location.highlightedRegion?.startLine,
token.location.highlightedRegion?.endLine,
token.location.highlightedRegion,
token.text,
);
}
@@ -275,8 +273,7 @@ function generateMarkdownForPathResults(
const threadFlow = codeFlow.threadFlows[i];
const link = createMarkdownRemoteFileRef(
threadFlow.fileLink,
threadFlow.highlightedRegion?.startLine,
threadFlow.highlightedRegion?.endLine,
threadFlow.highlightedRegion,
);
pathLines.push(`${listNumber}. ${link}`);

@@ -361,13 +358,18 @@ function generateMarkdownForRawTableCell(
*/
export function createMarkdownRemoteFileRef(
fileLink: FileLink,
startLine?: number,
endLine?: number,
region?: HighlightedRegion,
linkText?: string,
): string {
const markdownLink = `[${
linkText || fileLink.filePath
}](${createRemoteFileRef(fileLink, startLine, endLine)})`;
}](${createRemoteFileRef(
fileLink,
region?.startLine,
region?.endLine,
region?.startColumn,
region?.endColumn,
)})`;
return markdownLink;
}

Original file line number Diff line number Diff line change
@@ -73,6 +73,8 @@ export const CodeSnippetMessage = ({
token.location.fileLink,
token.location.highlightedRegion?.startLine,
token.location.highlightedRegion?.endLine,
token.location.highlightedRegion?.startColumn,
token.location.highlightedRegion?.endColumn,
)}
>
{token.text}
Original file line number Diff line number Diff line change
@@ -65,6 +65,8 @@ export const FileCodeSnippet = ({
fileLink,
highlightedRegion?.startLine || startingLine,
highlightedRegion?.endLine || endingLine,
highlightedRegion?.startColumn,
highlightedRegion?.endColumn,
);

if (!codeSnippet) {
Original file line number Diff line number Diff line change
@@ -87,7 +87,7 @@ describe("getting links to remote (GitHub) locations", () => {
);

expect(link).toEqual(
"https://github.com/owner/repo/blob/sha1234/path/to/file.ext#L194-L237",
"https://github.com/owner/repo/blob/sha1234/path/to/file.ext#L194C18-L237C1",
);
});

@@ -129,7 +129,7 @@ describe("getting links to remote (GitHub) locations", () => {
);

expect(link).toEqual(
"https://github.com/owner/repo/blob/sha1234/path/to/file.ext#L194-L237",
"https://github.com/owner/repo/blob/sha1234/path/to/file.ext#L194C18-L237C1",
);
});
});

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
### meteor/meteor

[npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L259)
[npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L259C28-L259C62)

<pre><code class="javascript"> if (isWindows()) {
//set for the current session and beyond
@@ -9,46 +9,46 @@
}
</code></pre>

*This shell command depends on an uncontrolled [absolute path](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/config.js#L39).*
*This shell command depends on an uncontrolled [absolute path](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/config.js#L39C20-L39C61).*

#### Paths

<details>
<summary>Path with 11 steps</summary>

1. [npm-packages/meteor-installer/config.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/config.js#L39)
1. [npm-packages/meteor-installer/config.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/config.js#L39C20-L39C61)
<pre><code class="javascript">
const meteorLocalFolder = '.meteor';
const meteorPath = <strong>path.resolve(rootPath, meteorLocalFolder)</strong>;

module.exports = {
</code></pre>

2. [npm-packages/meteor-installer/config.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/config.js#L39)
2. [npm-packages/meteor-installer/config.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/config.js#L39C7-L39C61)
<pre><code class="javascript">
const meteorLocalFolder = '.meteor';
const <strong>meteorPath = path.resolve(rootPath, meteorLocalFolder)</strong>;

module.exports = {
</code></pre>

3. [npm-packages/meteor-installer/config.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/config.js#L44)
3. [npm-packages/meteor-installer/config.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/config.js#L44C3-L44C13)
<pre><code class="javascript"> METEOR_LATEST_VERSION,
extractPath: rootPath,
<strong>meteorPath</strong>,
release: process.env.INSTALL_METEOR_VERSION || METEOR_LATEST_VERSION,
rootPath,
</code></pre>

4. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L12)
4. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L12C3-L12C13)
<pre><code class="javascript">const os = require('os');
const {
<strong>meteorPath</strong>,
release,
startedPath,
</code></pre>

5. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L11-L23)
5. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L11C7-L23C27)
<pre><code class="javascript">const tmp = require('tmp');
const os = require('os');
const <strong>{</strong>
@@ -68,47 +68,47 @@
const {
</code></pre>

6. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L259)
6. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L259C42-L259C52)
<pre><code class="javascript"> if (isWindows()) {
//set for the current session and beyond
child_process.execSync(`setx path "${<strong>meteorPath</strong>}/;%path%`);
return;
}
</code></pre>

7. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L259)
7. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L259C42-L259C52)
<pre><code class="javascript"> if (isWindows()) {
//set for the current session and beyond
child_process.execSync(`setx path "${<strong>meteorPath</strong>}/;%path%`);
return;
}
</code></pre>

8. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L259)
8. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L259C42-L259C52)
<pre><code class="javascript"> if (isWindows()) {
//set for the current session and beyond
child_process.execSync(`setx path "${<strong>meteorPath</strong>}/;%path%`);
return;
}
</code></pre>

9. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L259)
9. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L259C42-L259C52)
<pre><code class="javascript"> if (isWindows()) {
//set for the current session and beyond
child_process.execSync(`setx path "${<strong>meteorPath</strong>}/;%path%`);
return;
}
</code></pre>

10. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L259)
10. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L259C42-L259C52)
<pre><code class="javascript"> if (isWindows()) {
//set for the current session and beyond
child_process.execSync(`setx path "${<strong>meteorPath</strong>}/;%path%`);
return;
}
</code></pre>

11. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L259)
11. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L259C28-L259C62)
<pre><code class="javascript"> if (isWindows()) {
//set for the current session and beyond
child_process.execSync(<strong>`setx path "${meteorPath}/;%path%`</strong>);
@@ -121,15 +121,15 @@
<details>
<summary>Path with 2 steps</summary>

1. [npm-packages/meteor-installer/config.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/config.js#L39)
1. [npm-packages/meteor-installer/config.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/config.js#L39C20-L39C61)
<pre><code class="javascript">
const meteorLocalFolder = '.meteor';
const meteorPath = <strong>path.resolve(rootPath, meteorLocalFolder)</strong>;

module.exports = {
</code></pre>

2. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L259)
2. [npm-packages/meteor-installer/install.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/install.js#L259C28-L259C62)
<pre><code class="javascript"> if (isWindows()) {
//set for the current session and beyond
child_process.execSync(<strong>`setx path "${meteorPath}/;%path%`</strong>);
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
### github/codeql

[javascript/extractor/tests/regexp/input/multipart.js](https://github.com/github/codeql/blob/d094bbc06d063d0da8d0303676943c345e61de53/javascript/extractor/tests/regexp/input/multipart.js#L17-L20)
[javascript/extractor/tests/regexp/input/multipart.js](https://github.com/github/codeql/blob/d094bbc06d063d0da8d0303676943c345e61de53/javascript/extractor/tests/regexp/input/multipart.js#L17C6-L20C6)

<pre><code class="javascript">
var bad95 = new RegExp(
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
### meteor/meteor

[packages/deprecated/markdown/showdown.js](https://github.com/meteor/meteor/blob/53f3c4442d3542d3d2a012a854472a0d1bef9d12/packages/deprecated/markdown/showdown.js#L415)
[packages/deprecated/markdown/showdown.js](https://github.com/meteor/meteor/blob/53f3c4442d3542d3d2a012a854472a0d1bef9d12/packages/deprecated/markdown/showdown.js#L415C41-L415C48)

<pre><code class="javascript"> /g,hashElement);
*/
@@ -13,7 +13,7 @@

----------------------------------------

[packages/deprecated/markdown/showdown.js](https://github.com/meteor/meteor/blob/53f3c4442d3542d3d2a012a854472a0d1bef9d12/packages/deprecated/markdown/showdown.js#L523)
[packages/deprecated/markdown/showdown.js](https://github.com/meteor/meteor/blob/53f3c4442d3542d3d2a012a854472a0d1bef9d12/packages/deprecated/markdown/showdown.js#L523C58-L523C61)

<pre><code class="javascript"> // Build a regex to find HTML tags and comments. See Friedl's
// "Mastering Regular Expressions", 2nd Ed., pp. 200-201.
@@ -26,7 +26,7 @@

----------------------------------------

[tools/tests/apps/modules/imports/links/acorn/src/parseutil.js](https://github.com/meteor/meteor/blob/53f3c4442d3542d3d2a012a854472a0d1bef9d12/tools/tests/apps/modules/imports/links/acorn/src/parseutil.js#L9)
[tools/tests/apps/modules/imports/links/acorn/src/parseutil.js](https://github.com/meteor/meteor/blob/53f3c4442d3542d3d2a012a854472a0d1bef9d12/tools/tests/apps/modules/imports/links/acorn/src/parseutil.js#L9C24-L9C38)

<pre><code class="javascript">// ## Parser utilities

@@ -39,15 +39,15 @@ pp.strictDirective = function(start) {

----------------------------------------

[tools/tests/apps/modules/imports/links/acorn/src/parseutil.js](https://github.com/meteor/meteor/blob/53f3c4442d3542d3d2a012a854472a0d1bef9d12/tools/tests/apps/modules/imports/links/acorn/src/parseutil.js#L9)
[tools/tests/apps/modules/imports/links/acorn/src/parseutil.js](https://github.com/meteor/meteor/blob/53f3c4442d3542d3d2a012a854472a0d1bef9d12/tools/tests/apps/modules/imports/links/acorn/src/parseutil.js#L9C43-L9C57)

<pre><code class="javascript">const literal = /^(?:'((?:\\.|[^'])*?)'|"(<strong>(?:\\.|[^"])*?</strong>)")/</code></pre>

*This part of the regular expression may cause exponential backtracking on strings starting with '"' and containing many repetitions of '\!'.*

----------------------------------------

[app/src/main/AndroidManifest.xml](https://github.com/AlexRogalskiy/android-nrf-toolbox/blob/034cf3aa7d2a3a4145177de32546ca518a462a66/app/src/main/AndroidManifest.xml#L239-L249)
[app/src/main/AndroidManifest.xml](https://github.com/AlexRogalskiy/android-nrf-toolbox/blob/034cf3aa7d2a3a4145177de32546ca518a462a66/app/src/main/AndroidManifest.xml#L239C3-L249C15)

<pre><code class="javascript"> &lt;/service&gt;

Loading
Oops, something went wrong.