Skip to content

Commit 7f100df

Browse files
authored
Fix: fix off-by-one error in ast-grep parsing: convert 0-indexed line/column numbers to 1-indexed (#2738)
Fixes #2737 `ast-grep` uses 0-based indices in its file + column output. However, qlty expects 1-based indexes. This was leading to off-by-one errors. The trufflehog parser shows how to implement this correctly by adding one to everything. This also fixes another issue where invalid columns (0) would cause the `snippet` not to show in the output, as can be seen in the jest snapshot diff below. This bug was leading to pretty critical lint issues not being caught at work :( This PR fixes the appropriate part of the code and updates the tests. The `jest` snapshot / integration test below didn't have a snippet prior to the fix because `0` is not considered a valid column. So this also fixes that bug as well :)
1 parent 663aded commit 7f100df

File tree

2 files changed

+49
-10
lines changed

2 files changed

+49
-10
lines changed

qlty-check/src/parser/ast_grep.rs

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,12 @@ impl Parser for AstGrep {
6060
let location = Some(Location {
6161
path: ast_grep_issue.file.clone(),
6262
range: Some(Range {
63-
start_line: ast_grep_issue.range.start.line,
64-
start_column: ast_grep_issue.range.start.column,
65-
end_line: ast_grep_issue.range.end.line,
66-
end_column: ast_grep_issue.range.end.column,
63+
// ast-grep outputs 0-indexed line/column numbers,
64+
// but qlty expects 1-indexed values.
65+
start_line: ast_grep_issue.range.start.line + 1,
66+
start_column: ast_grep_issue.range.start.column + 1,
67+
end_line: ast_grep_issue.range.end.line + 1,
68+
end_column: ast_grep_issue.range.end.column + 1,
6769
..Default::default()
6870
}),
6971
});
@@ -104,6 +106,40 @@ impl Parser for AstGrep {
104106
mod test {
105107
use super::*;
106108

109+
#[test]
110+
fn parse_line_numbers_are_one_indexed() {
111+
// ast-grep outputs 0-indexed line/column numbers.
112+
// qlty expects 1-indexed. Verify the parser converts correctly.
113+
let input = r#"[{
114+
"text": "argument :foo, String",
115+
"range": {
116+
"byteOffset": { "start": 0, "end": 20 },
117+
"start": { "line": 0, "column": 0 },
118+
"end": { "line": 0, "column": 20 }
119+
},
120+
"file": "test.rb",
121+
"lines": "argument :foo, String",
122+
"charCount": { "leading": 0, "trailing": 0 },
123+
"language": "Ruby",
124+
"metaVariables": { "single": {}, "multi": {}, "transformed": {} },
125+
"ruleId": "test-rule",
126+
"severity": "error",
127+
"note": null,
128+
"message": "test message",
129+
"labels": []
130+
}]"#;
131+
132+
let issues = AstGrep::default().parse("ast-grep", input).unwrap();
133+
assert_eq!(issues.len(), 1);
134+
135+
let range = issues[0].location.as_ref().unwrap().range.as_ref().unwrap();
136+
// Line 0 in ast-grep should become line 1 in qlty
137+
assert_eq!(range.start_line, 1, "start_line should be 1-indexed");
138+
assert_eq!(range.end_line, 1, "end_line should be 1-indexed");
139+
assert_eq!(range.start_column, 1, "start_column should be 1-indexed");
140+
assert_eq!(range.end_column, 21, "end_column should be 1-indexed");
141+
}
142+
107143
#[test]
108144
fn parse() {
109145
let input = r###"
@@ -210,9 +246,10 @@ mod test {
210246
location:
211247
path: sample.ts
212248
range:
213-
startLine: 2
214-
endLine: 2
215-
endColumn: 26
249+
startLine: 3
250+
startColumn: 1
251+
endLine: 3
252+
endColumn: 27
216253
"#);
217254
}
218255
}

qlty-plugins/plugins/linters/ast-grep/fixtures/__snapshots__/basic_v0.37.0.shot

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,16 @@ exports[`linter=ast-grep fixture=basic version=0.37.0 1`] = `
99
"location": {
1010
"path": "sample.ts",
1111
"range": {
12-
"endColumn": 26,
13-
"endLine": 2,
14-
"startLine": 2,
12+
"endColumn": 27,
13+
"endLine": 3,
14+
"startColumn": 1,
15+
"startLine": 3,
1516
},
1617
},
1718
"message": "AST pattern match found: no-await-in-promise-all",
1819
"mode": "MODE_BLOCK",
1920
"ruleKey": "no-await-in-promise-all",
21+
"snippet": "Promise.all([await foo()]);",
2022
"snippetWithContext": "async function foo() {}
2123

2224
Promise.all([await foo()]);

0 commit comments

Comments
 (0)