From 91f1416e4302252a7c6d2a558c20ef1b339a625b Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Mon, 23 Dec 2024 13:17:32 +0100 Subject: [PATCH] fix(unstable): don't error on non-existing attrs or type attr --- cli/js/40_lint.js | 42 +++++++++++++++++++++----- cli/tools/lint/ast_buffer/buffer.rs | 13 ++++---- cli/tools/lint/ast_buffer/ts_estree.rs | 5 +-- tests/unit/lint_plugin_test.ts | 22 ++++++++++++++ 4 files changed, 66 insertions(+), 16 deletions(-) diff --git a/cli/js/40_lint.js b/cli/js/40_lint.js index 30b1f86884..7f54ffdc1c 100644 --- a/cli/js/40_lint.js +++ b/cli/js/40_lint.js @@ -2,6 +2,7 @@ // @ts-check +import { ATTR_BIN_NODE } from "ext:cli/40_lint_selector.js"; import { compileSelector, parseSelector, @@ -15,10 +16,10 @@ const { // Keep in sync with Rust // 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. -const AST_PROP_TYPE = 0; -const AST_PROP_PARENT = 1; -const AST_PROP_RANGE = 2; -const AST_PROP_LENGTH = 3; +const AST_PROP_TYPE = 1; +const AST_PROP_PARENT = 2; +const AST_PROP_RANGE = 3; +const AST_PROP_LENGTH = 4; // Keep in sync with Rust // Each node property is tagged with this enum to denote @@ -421,10 +422,12 @@ class MatchCtx { /** * @param {AstContext["buf"]} buf * @param {AstContext["strTable"]} strTable + * @param {AstContext["strByType"]} strByType */ - constructor(buf, strTable) { + constructor(buf, strTable, strByType) { this.buf = buf; this.strTable = strTable; + this.strByType = strByType; } /** @@ -452,7 +455,19 @@ class MatchCtx { getAttrPathValue(offset, propIds, idx) { 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; const _prop = buf[offset++]; const kind = buf[offset++]; @@ -499,7 +514,18 @@ class MatchCtx { hasAttrPath(offset, propIds, idx) { 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 (idx === propIds.length - 1) return true; @@ -736,7 +762,7 @@ function createAstContext(buf) { strByType, typeByStr, propByStr, - matcher: new MatchCtx(buf, strTable), + matcher: new MatchCtx(buf, strTable, strByType), }; setNodeGetters(ctx); diff --git a/cli/tools/lint/ast_buffer/buffer.rs b/cli/tools/lint/ast_buffer/buffer.rs index f37041eff2..d162ee3de1 100644 --- a/cli/tools/lint/ast_buffer/buffer.rs +++ b/cli/tools/lint/ast_buffer/buffer.rs @@ -204,11 +204,11 @@ impl SerializeCtx { prop_map: vec![0; prop_size + 1], }; - ctx.str_table.insert(""); + let empty_str = ctx.str_table.insert(""); // Placeholder node is always 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()); // Insert default props that are always present @@ -218,10 +218,11 @@ impl SerializeCtx { let length_str = ctx.str_table.insert("length"); // These values are expected to be in this order on the JS side - ctx.prop_map[0] = type_str; - ctx.prop_map[1] = parent_str; - ctx.prop_map[2] = range_str; - ctx.prop_map[3] = length_str; + ctx.prop_map[0] = empty_str; + ctx.prop_map[1] = type_str; + ctx.prop_map[2] = parent_str; + ctx.prop_map[3] = range_str; + ctx.prop_map[4] = length_str; ctx } diff --git a/cli/tools/lint/ast_buffer/ts_estree.rs b/cli/tools/lint/ast_buffer/ts_estree.rs index 599499aa8d..64dbd82cde 100644 --- a/cli/tools/lint/ast_buffer/ts_estree.rs +++ b/cli/tools/lint/ast_buffer/ts_estree.rs @@ -200,8 +200,8 @@ impl From for u8 { #[derive(Debug, Clone)] pub enum AstProp { - // Base, these three must be in sync with JS. The - // order here for these 3 fields is important. + // Base, these must be in sync with JS in the same order. + Invalid, Type, Parent, Range, @@ -318,6 +318,7 @@ pub enum AstProp { impl Display for AstProp { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let s = match self { + AstProp::Invalid => "__invalid__", // unused AstProp::Parent => "parent", AstProp::Range => "range", AstProp::Type => "type", diff --git a/tests/unit/lint_plugin_test.ts b/tests/unit/lint_plugin_test.ts index 9506c3e0a8..38a7e1b091 100644 --- a/tests/unit/lint_plugin_test.ts +++ b/tests/unit/lint_plugin_test.ts @@ -212,6 +212,28 @@ Deno.test("Plugin - visitor attr", () => { 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", () => { let result = testVisit( "foo(1); foo(1, 2);",