Skip to content

fix: exclude PHP scalar type-hint/cast keywords from ast --kind string #1965

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.

Sign up for GitHub

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

Open
carlos-alm wants to merge 1 commit into
base: fix/issue-1819-exported-consts-with-non-recognized
Choose a base branch
from
Open

fix: exclude PHP scalar type-hint/cast keywords from ast --kind string #1965

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
52 changes: 50 additions & 2 deletions crates/codegraph-core/src/extractors/helpers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,13 @@ pub struct LangAstConfig {
/// Single-char prefixes that can appear before string quotes (e.g. `r`, `b`, `f`, `u` for Python).
/// Multi-char combos like `rb`, `fr` are handled by stripping each char in sequence.
pub string_prefixes: &'static [char],
/// When true, a node is only classified if `node.is_named()` — guards against
/// anonymous grammar tokens whose `kind()` string collides with a named node
/// type mapped above (e.g. PHP's `primitive_type`/`cast_type` productions lex
/// the `string` scalar type-hint keyword as an unnamed token identical to the
/// named `string` literal node type). Mirrors the WASM-side
/// `astRequiresNamedNode()` guard in `ast-analysis/rules/index.ts`.
pub requires_named_node: bool,
}

// ── Per-language configs ─────────────────────────────────────────────────────
Expand All @@ -223,6 +230,7 @@ pub const PYTHON_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['\'', '"'],
string_prefixes: &['r', 'b', 'f', 'u', 'R', 'B', 'F', 'U'],
requires_named_node: false,
};

pub const GO_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -233,6 +241,7 @@ pub const GO_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"', '`'],
string_prefixes: &[],
requires_named_node: false,
};

pub const RUST_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -243,6 +252,7 @@ pub const RUST_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const JAVA_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -253,6 +263,7 @@ pub const JAVA_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const CSHARP_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -263,6 +274,7 @@ pub const CSHARP_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const RUBY_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -273,6 +285,7 @@ pub const RUBY_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &["regex"],
quote_chars: &['\'', '"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const PHP_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -283,6 +296,7 @@ pub const PHP_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['\'', '"'],
string_prefixes: &[],
requires_named_node: true,
};

pub const C_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -293,6 +307,7 @@ pub const C_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const CPP_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -303,6 +318,7 @@ pub const CPP_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"'],
string_prefixes: &['L', 'u', 'U', 'R'],
requires_named_node: false,
};

/// CUDA is a C++ superset; the tree-sitter-cuda grammar extends C++ with
Expand All @@ -317,6 +333,7 @@ pub const CUDA_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"'],
string_prefixes: &['L', 'u', 'U', 'R'],
requires_named_node: false,
};

pub const KOTLIN_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -327,6 +344,7 @@ pub const KOTLIN_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const SWIFT_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -337,6 +355,7 @@ pub const SWIFT_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const SCALA_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -347,6 +366,7 @@ pub const SCALA_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const BASH_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -357,6 +377,7 @@ pub const BASH_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"', '\''],
string_prefixes: &[],
requires_named_node: false,
};

pub const ELIXIR_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -367,6 +388,7 @@ pub const ELIXIR_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &["sigil"],
quote_chars: &['"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const LUA_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -377,6 +399,7 @@ pub const LUA_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['\'', '"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const DART_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -387,6 +410,7 @@ pub const DART_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['\'', '"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const ZIG_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -397,6 +421,7 @@ pub const ZIG_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const HASKELL_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -407,6 +432,7 @@ pub const HASKELL_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"', '\''],
string_prefixes: &[],
requires_named_node: false,
};

pub const OCAML_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -417,6 +443,7 @@ pub const OCAML_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"'],
string_prefixes: &[],
requires_named_node: false,
};

// F# string nodes in tree-sitter-fsharp surface under the `string` kind inside
Expand All @@ -429,6 +456,7 @@ pub const FSHARP_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"'],
string_prefixes: &[],
requires_named_node: false,
};

/// Objective-C string literals use the `@"..."` prefix. The shared
Expand All @@ -442,6 +470,7 @@ pub const OBJC_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const GLEAM_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -452,6 +481,7 @@ pub const GLEAM_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const JULIA_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -462,6 +492,7 @@ pub const JULIA_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const CLOJURE_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -472,6 +503,7 @@ pub const CLOJURE_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &["regex_lit"],
quote_chars: &['"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const ERLANG_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -482,6 +514,7 @@ pub const ERLANG_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const GROOVY_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -495,6 +528,7 @@ pub const GROOVY_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['\'', '"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const R_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -506,6 +540,7 @@ pub const R_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['\'', '"'],
string_prefixes: &[],
requires_named_node: false,
};

pub const SOLIDITY_AST_CONFIG: LangAstConfig = LangAstConfig {
Expand All @@ -516,6 +551,7 @@ pub const SOLIDITY_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"', '\''],
string_prefixes: &[],
requires_named_node: false,
};

/// Verilog/SystemVerilog AST config.
Expand All @@ -533,6 +569,7 @@ pub const VERILOG_AST_CONFIG: LangAstConfig = LangAstConfig {
regex_types: &[],
quote_chars: &['"'],
string_prefixes: &[],
requires_named_node: false,
};

// ── Generic AST node walker ──────────────────────────────────────────────────
Expand Down Expand Up @@ -565,7 +602,18 @@ pub fn walk_ast_nodes_with_config(

/// Classify a tree-sitter node against the language AST config.
/// Returns the AST kind string if matched, or `None` to skip.
fn classify_ast_node<'a>(kind: &str, config: &'a LangAstConfig) -> Option<&'a str> {
///
/// When `config.requires_named_node` is set, anonymous grammar tokens are
/// rejected even if their `kind()` string matches a mapped type — e.g. PHP's
/// `primitive_type`/`cast_type` productions lex the `string` scalar
/// type-hint keyword as an unnamed token whose `kind()` collides with the
/// named `string` literal node (#1821, mirrors TypeScript's `predefined_type`
/// guard from #1729).
fn classify_ast_node<'a>(node: &Node, config: &'a LangAstConfig) -> Option<&'a str> {
if config.requires_named_node && !node.is_named() {
return None;
}
let kind = node.kind();
if config.new_types.contains(&kind) {
Some("new")
} else if config.throw_types.contains(&kind) {
Expand Down Expand Up @@ -673,7 +721,7 @@ fn walk_ast_nodes_with_config_depth(
return;
}

if let Some(ast_kind) = classify_ast_node(node.kind(), config) {
if let Some(ast_kind) = classify_ast_node(node, config) {
match ast_kind {
"new" => {
ast_nodes.push(build_new_node(node, source));
Expand Down
43 changes: 30 additions & 13 deletions src/ast-analysis/rules/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,20 +383,37 @@ export function astStopRecurseKinds(langId: string): ReadonlySet<string> {

// ─── Per-language "named-node-required" guard ────────────────────────────
//
// tree-sitter-typescript's `predefined_type` production (the `string`,
// `number`, `boolean`, `void`, ... primitive type keywords) lexes its
// keyword as an anonymous grammar token whose `type` string is identical to
// the *named* `string` node type used for real string literals — the
// grammar's own node-types.json lists both a `"string", named: true` entry
// (the literal) and a `"string", named: false` entry (the keyword token).
// Matching by `node.type` alone therefore misclassifies `name: string`
// type annotations as string-literal ast_nodes (#1729). Mirrors the native
// `is_named()` guard in `extractors/javascript.rs::walk_ast_nodes_depth`.
// Some grammars reuse the same `type` string for two different node-types
// entries: a *named* node (a real literal) and an *anonymous* token (a
// keyword). Matching by `node.type` alone misclassifies the keyword as the
// literal. Two bundled grammars currently hit this:
//
// Scoped to the JS family: no other bundled grammar currently maps an
// AST_TYPE_MAPS key that collides with an anonymous token of the same name.
const JS_REQUIRES_NAMED_NODE: ReadonlySet<string> = new Set(['javascript', 'typescript', 'tsx']);
// - tree-sitter-typescript's `predefined_type` production (the `string`,
// `number`, `boolean`, `void`, ... primitive type keywords) lexes its
// keyword as an anonymous grammar token whose `type` string is identical
// to the *named* `string` node type used for real string literals —
// node-types.json lists both a `"string", named: true` entry (the
// literal) and a `"string", named: false` entry (the keyword token).
// `name: string` type annotations were misclassified as string-literal
// ast_nodes (#1729).
// - tree-sitter-php's `primitive_type`/`cast_type` productions (scalar
// type-hints like `string $x` and casts like `(string)`) lex the `string`
// keyword the same anonymous way, colliding with PHP's *named* `string`
// literal node type (#1821).
//
// Mirrors the native `is_named()`/`requires_named_node` guards in
// `extractors/javascript.rs::walk_ast_nodes_depth` (JS bespoke walker) and
// `extractors/helpers.rs::classify_ast_node` (generic walker, PHP config).
//
// A language is added here only when a `node-types.json` sweep confirms an
// AST_TYPE_MAPS key collides with an anonymous token of the same name.
const REQUIRES_NAMED_NODE: ReadonlySet<string> = new Set([
'javascript',
'typescript',
'tsx',
'php',
]);

export function astRequiresNamedNode(langId: string): boolean {
return JS_REQUIRES_NAMED_NODE.has(langId);
return REQUIRES_NAMED_NODE.has(langId);
}
5 changes: 4 additions & 1 deletion src/ast-analysis/visitors/ast-store-visitor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,10 @@ function collectNode(ctx: CollectCtx, node: TreeSitterNode, kind: string): void
* When `requireNamedNode` is set, anonymous grammar tokens are rejected even
* if their `type` string matches a mapped key — e.g. TypeScript's
* `predefined_type` keyword (`string`, `number`, ...) lexes as an unnamed
* token whose type collides with the named `string` literal node (#1729).
* token whose type collides with the named `string` literal node (#1729),
* and PHP's `primitive_type`/`cast_type` keyword (`string`) collides with
* the named `string` literal node the same way (#1821). See
* `astRequiresNamedNode()` in `../rules/index.ts` for the full language list.
*/
function resolveAstKind(
node: TreeSitterNode,
Expand Down
3 changes: 2 additions & 1 deletion tests/engines/ast-parity.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,10 +260,11 @@ end
ext: '.php',
code: `<?php
class App {
public function run() {
public function run(string $label): string {
$s = "hello world";
$o = new \\RuntimeException("bad");
throw $o;
return (string) $label;
}
}
`,
Expand Down
9 changes: 9 additions & 0 deletions tests/parsers/ast-all-langs.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,15 @@ describe.skipIf(!canTestMultiLang)('native AST nodes — multi-language', () =>
).toBe(true);
});

test('PHP: does not misclassify scalar type-hint keyword as kind:string (#1821)', () => {
// `string $name` (createUser's parameter, line 233) must never surface as a
// bare, unquoted "string" row — genuine literals are always quoted in `text`.
const strings = db
.prepare("SELECT * FROM ast_nodes WHERE kind = 'string' AND file LIKE '%fixture.php'")
.all();
expect(strings.some((n) => n.text === 'string')).toBe(false);
});

// ── Cross-language ──

test('all nodes have valid kinds', () => {
Expand Down
Loading
Loading