Skip to content

Commit

Permalink
fix incorrect yaml positions (#7946)
Browse files Browse the repository at this point in the history
YAML parsing was fixed in
#7918 and
#7931, but I didn't catch
that node start/end positions were incorrect. This was because in
`ctypes_read()` we were returning a JS int when we should have been
returning `UInt32` instead.

test plan:
- confirmed locally that the playground works properly w/ a yaml pattern
- added unit test to confirm yaml check works and returns correct
positions
- `dune build && make test` succeeded
  • Loading branch information
tpetr committed Jun 5, 2023
1 parent 3cc0c58 commit 6083bdb
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 3 deletions.
31 changes: 31 additions & 0 deletions js/engine/tests/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,34 @@ describe("engine", () => {
expect(engine.isMissingLanguages()).toBe(false);
});
});

describe("yaml parser", () => {
test("parses a pattern", async () => {
const engine = await enginePromise;
const result = JSON.parse(
engine.execute(
"yaml",
`${__dirname}/test-rule-yaml.json`,
`${__dirname}/test.yaml`
)
);

// it took a lot of work to get libyaml working, so let's confirm start/end locations are correct
expect(result.stats.okfiles).toBe(1);
expect(result.matches.length).toBe(1);
const match = result.matches[0];
expect(match.location.start).toEqual(
expect.objectContaining({ offset: 0, line: 1, col: 1 })
);
expect(match.location.end).toEqual(
expect.objectContaining({ offset: 8, line: 1, col: 9 })
);
expect(match.extra.metavars["$X"].start).toEqual(
expect.objectContaining({ offset: 5, line: 1, col: 6 })
);
expect(match.extra.metavars["$X"].end).toEqual(
expect.objectContaining({ offset: 8, line: 1, col: 9 })
);
expect(match.extra.metavars["$X"].abstract_content).toEqual("bar");
});
});
11 changes: 11 additions & 0 deletions js/engine/tests/test-rule-yaml.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"rules": [
{
"id": "test",
"languages": ["yaml"],
"pattern": "foo: $X",
"message": "test",
"severity": "ERROR"
}
]
}
1 change: 1 addition & 0 deletions js/engine/tests/test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
foo: bar
2 changes: 1 addition & 1 deletion js/libyaml/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function ctypes_read(primType, buffer) {
case 5: // Ctypes_Int
return libyaml.getValue(buffer[2], "i32");
case 13: // Ctypes_Size_t
return libyaml.getValue(buffer[2], "i32");
return new UInt32(libyaml.getValue(buffer[2], "i32"));
case 20: // Ctypes_Uint32_t
return new UInt32(libyaml.getValue(buffer[2], "i32"));
default:
Expand Down
3 changes: 1 addition & 2 deletions src/parsing/yaml_to_generic.ml
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,7 @@ let mk_bracket
_;
} ) v env =
(* The end index needs to be adjusted by one because the token is off *)
(* TODO: figure out why we get an off-by-one with jsoo *)
let e_index' = if !Common.jsoo then e_index else e_index - 1 in
let e_index' = e_index - 1 in
let e_line, e_column =
match env.charpos_to_pos with
| None -> (e_line, e_column)
Expand Down

0 comments on commit 6083bdb

Please sign in to comment.