0
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2025-02-01 12:16:11 -05:00

fix(unstable): don't error on non-existing attrs or type attr (#27456)

When running selectors for JS linting plugins we would error when
encountering an unknown attribute name:

```js
// selector
Foo[non-existant]

// error
Error: Missing string id: <number>
```

This was caused by using `0` as the invalid marker, but also overloading
`0` with an actual node type. So the fix is to reserve `0` as the
invalid marker and move the property type to the next index.
This commit is contained in:
Marvin Hagemeister 2024-12-27 22:46:29 +01:00 committed by GitHub
parent c45d0dadb3
commit fdd0edf23c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 65 additions and 16 deletions

View file

@ -15,10 +15,10 @@ const {
// Keep in sync with Rust // Keep in sync with Rust
// These types are expected to be present on every node. Note that this // These types are expected to be present on every node. Note that this
// isn't set in stone. We could revise this at a future point. // isn't set in stone. We could revise this at a future point.
const AST_PROP_TYPE = 0; const AST_PROP_TYPE = 1;
const AST_PROP_PARENT = 1; const AST_PROP_PARENT = 2;
const AST_PROP_RANGE = 2; const AST_PROP_RANGE = 3;
const AST_PROP_LENGTH = 3; const AST_PROP_LENGTH = 4;
// Keep in sync with Rust // Keep in sync with Rust
// Each node property is tagged with this enum to denote // Each node property is tagged with this enum to denote
@ -421,10 +421,12 @@ class MatchCtx {
/** /**
* @param {AstContext["buf"]} buf * @param {AstContext["buf"]} buf
* @param {AstContext["strTable"]} strTable * @param {AstContext["strTable"]} strTable
* @param {AstContext["strByType"]} strByType
*/ */
constructor(buf, strTable) { constructor(buf, strTable, strByType) {
this.buf = buf; this.buf = buf;
this.strTable = strTable; this.strTable = strTable;
this.strByType = strByType;
} }
/** /**
@ -452,7 +454,19 @@ class MatchCtx {
getAttrPathValue(offset, propIds, idx) { getAttrPathValue(offset, propIds, idx) {
const { buf } = this; const { buf } = this;
offset = findPropOffset(buf, offset, propIds[idx]); const propId = propIds[idx];
switch (propId) {
case AST_PROP_TYPE: {
const type = this.getType(offset);
return getString(this.strTable, this.strByType[type]);
}
case AST_PROP_PARENT:
case AST_PROP_RANGE:
throw new Error(`Not supported`);
}
offset = findPropOffset(buf, offset, propId);
if (offset === -1) return undefined; if (offset === -1) return undefined;
const _prop = buf[offset++]; const _prop = buf[offset++];
const kind = buf[offset++]; const kind = buf[offset++];
@ -499,7 +513,18 @@ class MatchCtx {
hasAttrPath(offset, propIds, idx) { hasAttrPath(offset, propIds, idx) {
const { buf } = this; const { buf } = this;
offset = findPropOffset(buf, offset, propIds[idx]); const propId = propIds[idx];
// If propId is 0 then the property doesn't exist in the AST
if (propId === 0) return false;
switch (propId) {
case AST_PROP_TYPE:
case AST_PROP_PARENT:
case AST_PROP_RANGE:
return true;
}
offset = findPropOffset(buf, offset, propId);
if (offset === -1) return false; if (offset === -1) return false;
if (idx === propIds.length - 1) return true; if (idx === propIds.length - 1) return true;
@ -736,7 +761,7 @@ function createAstContext(buf) {
strByType, strByType,
typeByStr, typeByStr,
propByStr, propByStr,
matcher: new MatchCtx(buf, strTable), matcher: new MatchCtx(buf, strTable, strByType),
}; };
setNodeGetters(ctx); setNodeGetters(ctx);

View file

@ -204,11 +204,11 @@ impl SerializeCtx {
prop_map: vec![0; prop_size + 1], prop_map: vec![0; prop_size + 1],
}; };
ctx.str_table.insert(""); let empty_str = ctx.str_table.insert("");
// Placeholder node is always 0 // Placeholder node is always 0
ctx.append_node(0, NodeRef(0), &DUMMY_SP, 0); ctx.append_node(0, NodeRef(0), &DUMMY_SP, 0);
ctx.kind_map[0] = 0; ctx.kind_map[0] = empty_str;
ctx.start_buf = NodeRef(ctx.buf.len()); ctx.start_buf = NodeRef(ctx.buf.len());
// Insert default props that are always present // Insert default props that are always present
@ -218,10 +218,11 @@ impl SerializeCtx {
let length_str = ctx.str_table.insert("length"); let length_str = ctx.str_table.insert("length");
// These values are expected to be in this order on the JS side // These values are expected to be in this order on the JS side
ctx.prop_map[0] = type_str; ctx.prop_map[0] = empty_str;
ctx.prop_map[1] = parent_str; ctx.prop_map[1] = type_str;
ctx.prop_map[2] = range_str; ctx.prop_map[2] = parent_str;
ctx.prop_map[3] = length_str; ctx.prop_map[3] = range_str;
ctx.prop_map[4] = length_str;
ctx ctx
} }

View file

@ -200,8 +200,8 @@ impl From<AstNode> for u8 {
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub enum AstProp { pub enum AstProp {
// Base, these three must be in sync with JS. The // Base, these must be in sync with JS in the same order.
// order here for these 3 fields is important. Invalid,
Type, Type,
Parent, Parent,
Range, Range,
@ -318,6 +318,7 @@ pub enum AstProp {
impl Display for AstProp { impl Display for AstProp {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let s = match self { let s = match self {
AstProp::Invalid => "__invalid__", // unused
AstProp::Parent => "parent", AstProp::Parent => "parent",
AstProp::Range => "range", AstProp::Range => "range",
AstProp::Type => "type", AstProp::Type => "type",

View file

@ -212,6 +212,28 @@ Deno.test("Plugin - visitor attr", () => {
assertEquals(result[0].node.name, "foo"); assertEquals(result[0].node.name, "foo");
}); });
Deno.test("Plugin - visitor attr to check type", () => {
let result = testVisit(
"foo",
"Identifier[type]",
);
assertEquals(result[0].node.type, "Identifier");
result = testVisit(
"foo",
"Identifier[type='Identifier']",
);
assertEquals(result[0].node.type, "Identifier");
});
Deno.test("Plugin - visitor attr non-existing", () => {
const result = testVisit(
"foo",
"[non-existing]",
);
assertEquals(result, []);
});
Deno.test("Plugin - visitor attr length special case", () => { Deno.test("Plugin - visitor attr length special case", () => {
let result = testVisit( let result = testVisit(
"foo(1); foo(1, 2);", "foo(1); foo(1, 2);",