From 5ad6e47b8ec3b5a8a4971586fa6a84b98555e2cd Mon Sep 17 00:00:00 2001 From: Guilherme Souza Date: Thu, 25 Jun 2026 13:59:34 -0300 Subject: [PATCH 1/3] feat(api-check): include file path and line number in compliance failure messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parsers now emit a `line` field on each `ParsedSymbol` so the CI check can show exactly where in the codebase a new public symbol was declared. - `ParsedSymbol` gains an optional `line?: number` field - TypeScript parser computes 1-based line numbers from the TS AST - Swift SymbolGraph normalizer extracts `position.line` (0→1-based) - Griffe normalizer passes `lineno` from each Griffe node - Dart extractor threads `LineInfo` through and captures line numbers for every class, member, function, and variable declaration - `CheckResult.uncoveredSymbols` is now `ParsedSymbol[]` (was `string[]`) so callers retain the file/line context through to the formatter - `formatErrorMessage` renders `defined at: :` under each symbol when location info is available Error output now matches the desired shape from SDK-1092: - SupabaseClient.signInWithPasskey (supabase-flutter) defined at: Sources/Auth/SupabaseClient.swift:142 Linear: SDK-1092 --- scripts/capability-matrix/src/api-check.ts | 25 ++++++++----- .../capability-matrix/src/normalize-griffe.ts | 10 +++--- .../src/normalize-symbolgraph.ts | 10 ++++-- scripts/capability-matrix/src/ts-parser.ts | 18 +++++++--- .../capability-matrix/test/api-check.test.ts | 34 ++++++++++++++---- .../test/normalize-griffe.test.ts | 30 ++++++++++++++++ .../test/normalize-symbolgraph.test.ts | 34 +++++++++++++++--- .../capability-matrix/test/ts-parser.test.ts | 18 ++++++++-- .../lib/src/extractor.dart | 35 +++++++++++++------ .../lib/src/parsed_symbol.dart | 4 ++- .../test/extractor_test.dart | 17 +++++++++ 11 files changed, 189 insertions(+), 46 deletions(-) diff --git a/scripts/capability-matrix/src/api-check.ts b/scripts/capability-matrix/src/api-check.ts index 7952508..19485b9 100644 --- a/scripts/capability-matrix/src/api-check.ts +++ b/scripts/capability-matrix/src/api-check.ts @@ -4,7 +4,7 @@ import type { ParsedSymbol } from "./ts-parser.js"; export interface CheckResult { newSymbols: string[]; - uncoveredSymbols: string[]; + uncoveredSymbols: ParsedSymbol[]; removedRegisteredSymbols: Array<{ symbol: string; featureId: string }>; } @@ -16,12 +16,11 @@ export function checkNewSymbols( const baseNames = new Set(baseSymbols.map((s) => s.name)); const prNames = new Set(prSymbols.map((s) => s.name)); - const newSymbols = prSymbols - .filter((s) => !baseNames.has(s.name)) - .map((s) => s.name); + const newSymbolObjs = prSymbols.filter((s) => !baseNames.has(s.name)); + const newSymbols = newSymbolObjs.map((s) => s.name); const symbolIndex = buildSymbolIndex(compliance); - const uncoveredSymbols = newSymbols.filter((sym) => !symbolIndex.has(sym)); + const uncoveredSymbols = newSymbolObjs.filter((s) => !symbolIndex.has(s.name)); const removedRegisteredSymbols = baseSymbols .filter((s) => !prNames.has(s.name) && symbolIndex.has(s.name)) @@ -31,24 +30,32 @@ export function checkNewSymbols( } export function formatErrorMessage( - uncoveredSymbols: string[], + uncoveredSymbols: ParsedSymbol[], sdkName: string, ): string { const lines: string[] = [ "❌ Capability matrix check failed", "New public API detected that is not in the capability matrix:", - ...uncoveredSymbols.map((s) => ` - ${s} (${sdkName})`), + ]; + for (const s of uncoveredSymbols) { + lines.push(` - ${s.name} (${sdkName})`); + if (s.file) { + const location = s.line !== undefined ? `${s.file}:${s.line}` : s.file; + lines.push(` defined at: ${location}`); + } + } + lines.push( "", "Register each symbol in sdk-compliance.yaml under the appropriate feature:", "", " auth.my_feature:", " status: implemented", " symbols:", - ` - ${uncoveredSymbols[0] ?? "ClassName.methodName"}`, + ` - ${uncoveredSymbols[0]?.name ?? "ClassName.methodName"}`, "", "If the feature does not exist in the matrix yet, add it there first:", " https://github.com/supabase/sdk/blob/main/CONTRIBUTING.md", - ]; + ); return lines.join("\n"); } diff --git a/scripts/capability-matrix/src/normalize-griffe.ts b/scripts/capability-matrix/src/normalize-griffe.ts index 6fb47a6..384154a 100644 --- a/scripts/capability-matrix/src/normalize-griffe.ts +++ b/scripts/capability-matrix/src/normalize-griffe.ts @@ -9,6 +9,7 @@ export interface GriffeNode { filepath?: string; labels?: string[] | null; members?: Record; + lineno?: number; } export type GriffeOutput = Record; @@ -47,7 +48,7 @@ function walkNode( if (name.startsWith("_")) return; if (node.kind === "class") { - emit(symbols, ig, { name: qual(classStack, name), kind: "class", file }); + emit(symbols, ig, { name: qual(classStack, name), kind: "class", file }, node.lineno); for (const [childName, child] of Object.entries(node.members ?? {})) { walkNode(childName, child, file, [...classStack, name], symbols, ig, projectRoot); } @@ -56,12 +57,12 @@ function walkNode( if (node.kind === "function") { const kind = classStack.length > 0 ? ("method" as const) : ("function" as const); - emit(symbols, ig, { name: qual(classStack, name), kind, file }); + emit(symbols, ig, { name: qual(classStack, name), kind, file }, node.lineno); return; } if (node.kind === "attribute" && node.labels?.includes("property")) { - emit(symbols, ig, { name: qual(classStack, name), kind: "property", file }); + emit(symbols, ig, { name: qual(classStack, name), kind: "property", file }, node.lineno); } } @@ -69,7 +70,8 @@ function qual(classStack: string[], name: string): string { return classStack.length > 0 ? `${classStack.join(".")}.${name}` : name; } -function emit(symbols: ParsedSymbol[], ig: Ignore | null, sym: ParsedSymbol): void { +function emit(symbols: ParsedSymbol[], ig: Ignore | null, sym: ParsedSymbol, lineno?: number): void { if (ig && sym.file && ig.ignores(sym.file)) return; + if (lineno !== undefined) sym.line = lineno; symbols.push(sym); } diff --git a/scripts/capability-matrix/src/normalize-symbolgraph.ts b/scripts/capability-matrix/src/normalize-symbolgraph.ts index e1cd95b..b57cd3e 100644 --- a/scripts/capability-matrix/src/normalize-symbolgraph.ts +++ b/scripts/capability-matrix/src/normalize-symbolgraph.ts @@ -5,7 +5,7 @@ export type { ParsedSymbol, ParseResult }; export interface SymbolGraphSymbol { kind: { identifier: string }; pathComponents: string[]; - location?: { uri: string }; + location?: { uri: string; position?: { line: number; character: number } }; } // Kind identifiers that map to ParsedSymbol kinds. @@ -41,11 +41,15 @@ export function normalizeSymbolGraph( const kind = KIND_MAP[sym.kind.identifier]; if (kind === undefined) continue; - result.push({ + const sym_: ParsedSymbol = { name: qualifiedName(sym.pathComponents), kind, file: resolveFile(sym.location?.uri, sdkRoot), - }); + }; + if (sym.location?.position !== undefined) { + sym_.line = sym.location.position.line + 1; + } + result.push(sym_); } return { symbols: result }; diff --git a/scripts/capability-matrix/src/ts-parser.ts b/scripts/capability-matrix/src/ts-parser.ts index cbe6d31..5846ba7 100644 --- a/scripts/capability-matrix/src/ts-parser.ts +++ b/scripts/capability-matrix/src/ts-parser.ts @@ -7,6 +7,7 @@ export interface ParsedSymbol { name: string; kind: "class" | "method" | "property" | "function" | "variable"; file: string; + line?: number; } export interface ParseResult { @@ -65,6 +66,7 @@ function extractClassMembers( className: string, node: ts.ClassDeclaration, relPath: string, + sf: ts.SourceFile, out: ParsedSymbol[], ): void { for (const member of node.members) { @@ -81,7 +83,8 @@ function extractClassMembers( ? "method" : "property"; - out.push({ name: `${className}.${name}`, kind, file: relPath }); + const line = sf.getLineAndCharacterOfPosition(member.getStart(sf)).line + 1; + out.push({ name: `${className}.${name}`, kind, file: relPath, line }); } } @@ -100,16 +103,21 @@ export function extractFromSource( if (ts.isClassDeclaration(stmt) && isExported(stmt)) { const className = stmt.name?.text; if (className) { - symbols.push({ name: className, kind: "class", file: relPath }); - extractClassMembers(className, stmt, relPath, symbols); + const line = sf.getLineAndCharacterOfPosition(stmt.getStart(sf)).line + 1; + symbols.push({ name: className, kind: "class", file: relPath, line }); + extractClassMembers(className, stmt, relPath, sf, symbols); } } else if (ts.isFunctionDeclaration(stmt) && isExported(stmt)) { const name = stmt.name?.text; - if (name) symbols.push({ name, kind: "function", file: relPath }); + if (name) { + const line = sf.getLineAndCharacterOfPosition(stmt.getStart(sf)).line + 1; + symbols.push({ name, kind: "function", file: relPath, line }); + } } else if (ts.isVariableStatement(stmt) && isExported(stmt)) { for (const decl of stmt.declarationList.declarations) { if (ts.isIdentifier(decl.name)) { - symbols.push({ name: decl.name.text, kind: "variable", file: relPath }); + const line = sf.getLineAndCharacterOfPosition(decl.getStart(sf)).line + 1; + symbols.push({ name: decl.name.text, kind: "variable", file: relPath, line }); } } } diff --git a/scripts/capability-matrix/test/api-check.test.ts b/scripts/capability-matrix/test/api-check.test.ts index 4fc60f1..0248b3c 100644 --- a/scripts/capability-matrix/test/api-check.test.ts +++ b/scripts/capability-matrix/test/api-check.test.ts @@ -2,8 +2,10 @@ import { describe, it, expect } from "vitest"; import { checkNewSymbols, formatErrorMessage, formatRemovedMessage } from "../src/api-check"; import type { ParsedSymbol } from "../src/ts-parser"; -function sym(name: string): ParsedSymbol { - return { name, kind: "method", file: "src/index.ts" }; +function sym(name: string, line?: number): ParsedSymbol { + const s: ParsedSymbol = { name, kind: "method", file: "src/index.ts" }; + if (line !== undefined) s.line = line; + return s; } const compliance = { @@ -34,7 +36,7 @@ describe("checkNewSymbols", () => { const pr = [sym("AuthClient.signInWithPasskey")]; const result = checkNewSymbols(base, pr, compliance); expect(result.newSymbols).toEqual(["AuthClient.signInWithPasskey"]); - expect(result.uncoveredSymbols).toEqual(["AuthClient.signInWithPasskey"]); + expect(result.uncoveredSymbols).toEqual([sym("AuthClient.signInWithPasskey")]); }); it("ignores symbols that exist in both base and PR", () => { @@ -57,7 +59,7 @@ describe("checkNewSymbols", () => { const base: ParsedSymbol[] = []; const pr = [sym("AuthClient.foo"), sym("AuthClient.bar")]; const result = checkNewSymbols(base, pr, compliance); - expect(result.uncoveredSymbols).toEqual(["AuthClient.foo", "AuthClient.bar"]); + expect(result.uncoveredSymbols).toEqual([sym("AuthClient.foo"), sym("AuthClient.bar")]); }); }); @@ -98,17 +100,37 @@ describe("checkNewSymbols — removed registered symbols", () => { describe("formatErrorMessage", () => { it("includes all uncovered symbols in output", () => { - const msg = formatErrorMessage(["AuthClient.signInWithPasskey"], "javascript"); + const msg = formatErrorMessage([sym("AuthClient.signInWithPasskey")], "javascript"); expect(msg).toContain("❌ Capability matrix check failed"); expect(msg).toContain("AuthClient.signInWithPasskey (javascript)"); expect(msg).toContain("sdk-compliance.yaml"); }); it("includes multiple uncovered symbols", () => { - const msg = formatErrorMessage(["Foo.a", "Foo.b"], "flutter"); + const msg = formatErrorMessage([sym("Foo.a"), sym("Foo.b")], "flutter"); expect(msg).toContain("Foo.a (flutter)"); expect(msg).toContain("Foo.b (flutter)"); }); + + it("includes defined-at with file and line when both are present", () => { + const s: ParsedSymbol = { name: "SupabaseClient.signInWithPasskey", kind: "method", file: "Sources/Auth/SupabaseClient.swift", line: 142 }; + const msg = formatErrorMessage([s], "swift"); + expect(msg).toContain("SupabaseClient.signInWithPasskey (swift)"); + expect(msg).toContain("defined at: Sources/Auth/SupabaseClient.swift:142"); + }); + + it("includes defined-at with file only when line is absent", () => { + const s: ParsedSymbol = { name: "AuthClient.signUp", kind: "method", file: "src/auth.ts" }; + const msg = formatErrorMessage([s], "javascript"); + expect(msg).toContain("defined at: src/auth.ts"); + expect(msg).not.toContain("defined at: src/auth.ts:"); + }); + + it("omits defined-at when file is empty", () => { + const s: ParsedSymbol = { name: "AuthClient.signUp", kind: "method", file: "" }; + const msg = formatErrorMessage([s], "javascript"); + expect(msg).not.toContain("defined at:"); + }); }); describe("formatRemovedMessage", () => { diff --git a/scripts/capability-matrix/test/normalize-griffe.test.ts b/scripts/capability-matrix/test/normalize-griffe.test.ts index 1a0a5a6..405c733 100644 --- a/scripts/capability-matrix/test/normalize-griffe.test.ts +++ b/scripts/capability-matrix/test/normalize-griffe.test.ts @@ -256,6 +256,36 @@ describe("normalizeGriffe — file inheritance", () => { }); }); +describe("normalizeGriffe — line numbers", () => { + it("includes line number when node has lineno", () => { + const input = pkg("/repo/src/client.py", { + MyClient: { kind: "class", members: {}, lineno: 10 }, + }); + const { symbols } = normalizeGriffe(input, "/repo"); + expect(symbols[0].line).toBe(10); + }); + + it("omits line when node has no lineno", () => { + const input = pkg("/repo/src/client.py", { + MyClient: { kind: "class", members: {} }, + }); + const { symbols } = normalizeGriffe(input, "/repo"); + expect(symbols[0].line).toBeUndefined(); + }); + + it("passes lineno through for methods", () => { + const input = pkg("/repo/src/client.py", { + MyClient: { + kind: "class", + members: { sign_up: { kind: "function", labels: null, lineno: 25 } }, + }, + }); + const { symbols } = normalizeGriffe(input, "/repo"); + const method = symbols.find((s) => s.name === "MyClient.sign_up"); + expect(method?.line).toBe(25); + }); +}); + describe("normalizeGriffe — .sdk-parse-ignore", () => { it("filters out symbols whose file matches .sdk-parse-ignore", () => { const root = mkdtempSync(join(tmpdir(), "griffe-test-")); diff --git a/scripts/capability-matrix/test/normalize-symbolgraph.test.ts b/scripts/capability-matrix/test/normalize-symbolgraph.test.ts index 78d4c5c..a60661d 100644 --- a/scripts/capability-matrix/test/normalize-symbolgraph.test.ts +++ b/scripts/capability-matrix/test/normalize-symbolgraph.test.ts @@ -11,12 +11,12 @@ function sym( identifier: string, pathComponents: string[], uri?: string, + position?: { line: number; character: number }, ): SymbolGraphSymbol { - return { - kind: { identifier }, - pathComponents, - ...(uri ? { location: { uri } } : {}), - }; + if (uri) { + return { kind: { identifier }, pathComponents, location: { uri, ...(position ? { position } : {}) } }; + } + return { kind: { identifier }, pathComponents }; } // --------------------------------------------------------------------------- @@ -163,6 +163,30 @@ describe("file path resolution", () => { }); }); +describe("line number extraction", () => { + it("extracts 1-based line from 0-based position.line", () => { + const sdkRoot = "/sdk"; + const uri = `file://${sdkRoot}/Sources/Auth/AuthClient.swift`; + const { symbols } = normalizeSymbolGraph( + [sym("swift.class", ["AuthClient"], uri, { line: 141, character: 0 })], + sdkRoot, + ); + expect(symbols[0].line).toBe(142); + }); + + it("omits line when position is absent", () => { + const sdkRoot = "/sdk"; + const uri = `file://${sdkRoot}/Sources/Auth/AuthClient.swift`; + const { symbols } = normalizeSymbolGraph([sym("swift.class", ["AuthClient"], uri)], sdkRoot); + expect(symbols[0].line).toBeUndefined(); + }); + + it("omits line when location is absent", () => { + const { symbols } = normalizeSymbolGraph([sym("swift.class", ["AuthClient"])], "/sdk"); + expect(symbols[0].line).toBeUndefined(); + }); +}); + // --------------------------------------------------------------------------- // Multi-input // --------------------------------------------------------------------------- diff --git a/scripts/capability-matrix/test/ts-parser.test.ts b/scripts/capability-matrix/test/ts-parser.test.ts index 648cbea..94a9f55 100644 --- a/scripts/capability-matrix/test/ts-parser.test.ts +++ b/scripts/capability-matrix/test/ts-parser.test.ts @@ -53,13 +53,27 @@ describe("extractFromSource", () => { it("extracts exported functions", () => { const source = `export function createClient(url: string): void {}`; const symbols = extractFromSource(source, "src/index.ts"); - expect(symbols).toEqual([{ name: "createClient", kind: "function", file: "src/index.ts" }]); + expect(symbols).toEqual([{ name: "createClient", kind: "function", file: "src/index.ts", line: 1 }]); }); it("extracts exported variables", () => { const source = `export const version = "1.0.0";`; const symbols = extractFromSource(source, "src/index.ts"); - expect(symbols).toEqual([{ name: "version", kind: "variable", file: "src/index.ts" }]); + expect(symbols).toEqual([{ name: "version", kind: "variable", file: "src/index.ts", line: 1 }]); + }); + + it("records line numbers for class and its members", () => { + const source = [ + "export class AuthClient {", + " public signUp(email: string): void {}", + " public signIn(): void {}", + "}", + ].join("\n"); + const symbols = extractFromSource(source, "src/index.ts"); + const byName = Object.fromEntries(symbols.map((s) => [s.name, s])); + expect(byName["AuthClient"].line).toBe(1); + expect(byName["AuthClient.signUp"].line).toBe(2); + expect(byName["AuthClient.signIn"].line).toBe(3); }); it("skips constructor", () => { diff --git a/scripts/dart_symbol_extractor/lib/src/extractor.dart b/scripts/dart_symbol_extractor/lib/src/extractor.dart index c087ce8..1961036 100644 --- a/scripts/dart_symbol_extractor/lib/src/extractor.dart +++ b/scripts/dart_symbol_extractor/lib/src/extractor.dart @@ -2,6 +2,7 @@ import 'dart:io'; import 'package:analyzer/dart/analysis/utilities.dart'; import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/source/line_info.dart'; import 'package:path/path.dart' as p; import 'ignore_matcher.dart'; @@ -19,14 +20,16 @@ const _generatedSuffixes = ['.g.dart', '.freezed.dart', '.gr.dart']; /// per file without following exports. List extractFromSource(String source, String relPath) { final symbols = []; - final unit = parseString( + final result = parseString( content: source, path: relPath, throwIfDiagnostics: false, - ).unit; + ); + final unit = result.unit; + final lineInfo = unit.lineInfo; for (final declaration in unit.declarations) { - _visitTopLevel(declaration, relPath, symbols); + _visitTopLevel(declaration, relPath, lineInfo, symbols); } return symbols; } @@ -34,8 +37,10 @@ List extractFromSource(String source, String relPath) { void _visitTopLevel( CompilationUnitMember declaration, String relPath, + LineInfo lineInfo, List out, ) { + final line = lineInfo.getLocation(declaration.offset).lineNumber; switch (declaration) { // Class-like containers expose their name and members identically. Unnamed // extensions (name == null) fall through, as they have no qualifiable @@ -45,36 +50,39 @@ void _visitTopLevel( case EnumDeclaration(:final name, :final members): case ExtensionTypeDeclaration(:final name, :final members): case ExtensionDeclaration(name: final name?, :final members): - _emitContainer(name.lexeme, members, relPath, out); + _emitContainer(name.lexeme, members, relPath, lineInfo, line, out); case FunctionDeclaration(:final name, :final isGetter, :final isSetter) when !isGetter && !isSetter: - _emit(name.lexeme, SymbolKind.function, relPath, out); + _emit(name.lexeme, SymbolKind.function, relPath, line, out); // `class C = A with M;` is a class, not a typedef, so it precedes TypeAlias. case ClassTypeAlias(:final name): - _emit(name.lexeme, SymbolKind.classKind, relPath, out); + _emit(name.lexeme, SymbolKind.classKind, relPath, line, out); case TypeAlias(:final name): - _emit(name.lexeme, SymbolKind.variable, relPath, out); + _emit(name.lexeme, SymbolKind.variable, relPath, line, out); case TopLevelVariableDeclaration(:final variables): for (final variable in variables.variables) { - _emit(variable.name.lexeme, SymbolKind.variable, relPath, out); + final varLine = lineInfo.getLocation(variable.offset).lineNumber; + _emit(variable.name.lexeme, SymbolKind.variable, relPath, varLine, out); } } } void _emit( - String name, SymbolKind kind, String relPath, List out) { + String name, SymbolKind kind, String relPath, int line, List out) { if (_isPrivate(name)) return; - out.add(ParsedSymbol(name: name, kind: kind, file: relPath)); + out.add(ParsedSymbol(name: name, kind: kind, file: relPath, line: line)); } void _emitContainer( String containerName, List members, String relPath, + LineInfo lineInfo, + int containerLine, List out, ) { if (_isPrivate(containerName)) return; @@ -83,10 +91,12 @@ void _emitContainer( name: containerName, kind: SymbolKind.classKind, file: relPath, + line: containerLine, ), ); for (final member in members) { + final memberLine = lineInfo.getLocation(member.offset).lineNumber; if (member is MethodDeclaration) { final name = member.name.lexeme; if (_isPrivate(name)) continue; @@ -94,17 +104,19 @@ void _emitContainer( ? SymbolKind.property : SymbolKind.method; out.add( - ParsedSymbol(name: '$containerName.$name', kind: kind, file: relPath), + ParsedSymbol(name: '$containerName.$name', kind: kind, file: relPath, line: memberLine), ); } else if (member is FieldDeclaration) { for (final field in member.fields.variables) { final name = field.name.lexeme; if (_isPrivate(name)) continue; + final fieldLine = lineInfo.getLocation(field.offset).lineNumber; out.add( ParsedSymbol( name: '$containerName.$name', kind: SymbolKind.property, file: relPath, + line: fieldLine, ), ); } @@ -118,6 +130,7 @@ void _emitContainer( name: '$containerName.$ctorName', kind: SymbolKind.method, file: relPath, + line: memberLine, ), ); } diff --git a/scripts/dart_symbol_extractor/lib/src/parsed_symbol.dart b/scripts/dart_symbol_extractor/lib/src/parsed_symbol.dart index a0bb75d..3e70e52 100644 --- a/scripts/dart_symbol_extractor/lib/src/parsed_symbol.dart +++ b/scripts/dart_symbol_extractor/lib/src/parsed_symbol.dart @@ -6,15 +6,17 @@ enum SymbolKind { classKind, method, property, function, variable } class ParsedSymbol { - ParsedSymbol({required this.name, required this.kind, required this.file}); + ParsedSymbol({required this.name, required this.kind, required this.file, this.line}); final String name; final SymbolKind kind; final String file; + final int? line; Map toJson() => { 'name': name, 'kind': kind == SymbolKind.classKind ? 'class' : kind.name, 'file': file, + if (line != null) 'line': line, }; } diff --git a/scripts/dart_symbol_extractor/test/extractor_test.dart b/scripts/dart_symbol_extractor/test/extractor_test.dart index 5476e7b..3bee005 100644 --- a/scripts/dart_symbol_extractor/test/extractor_test.dart +++ b/scripts/dart_symbol_extractor/test/extractor_test.dart @@ -43,6 +43,15 @@ class SupabaseClient { _byName(symbols, 'SupabaseClient.SupabaseClient').kind, SymbolKind.method, ); + // All symbols should have a non-null, positive line number. + for (final sym in symbols) { + expect(sym.line, isNotNull, reason: '${sym.name} should have a line number'); + expect(sym.line, greaterThan(0), reason: '${sym.name} line should be > 0'); + } + // Spot-check that SupabaseClient is on line 1 (first non-empty line) and + // signIn follows the setter on line 7. + expect(_byName(symbols, 'SupabaseClient').line, 1); + expect(_byName(symbols, 'SupabaseClient.signIn').line, 7); }); test('skips private declarations and private members', () { @@ -173,6 +182,14 @@ typedef Json = Map; 'packages/auth/lib/auth.dart', ); }); + + test('includes non-null line numbers for all symbols', () { + final symbols = parseDartProject('test/fixtures/sample_project'); + for (final sym in symbols) { + expect(sym.line, isNotNull, reason: '${sym.name} should have a line number'); + expect(sym.line, greaterThan(0)); + } + }); }); group('IgnoreMatcher', () { From dc566441710503cbefaf1c31e961f03d77a0b869 Mon Sep 17 00:00:00 2001 From: Guilherme Souza Date: Thu, 25 Jun 2026 14:17:36 -0300 Subject: [PATCH 2/3] fix(api-check): address code review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use firstTokenAfterCommentAndMetadata.offset in Dart extractor so annotated declarations report the line of the keyword, not the @annotation - Remove mutation in normalize-griffe emit(); use object spread instead - Rename sym_ → parsed in normalize-symbolgraph for clarity - Document that Griffe lineno is already 1-based (unlike TS/Swift) - Add test: line survives through checkNewSymbols into uncoveredSymbols - Add test: annotated Dart methods report the declaration keyword line --- .../capability-matrix/src/normalize-griffe.ts | 4 ++-- .../src/normalize-symbolgraph.ts | 6 ++--- .../capability-matrix/test/api-check.test.ts | 8 +++++++ .../lib/src/extractor.dart | 4 ++-- .../test/extractor_test.dart | 22 +++++++++++++++++++ 5 files changed, 37 insertions(+), 7 deletions(-) diff --git a/scripts/capability-matrix/src/normalize-griffe.ts b/scripts/capability-matrix/src/normalize-griffe.ts index 384154a..562ddc5 100644 --- a/scripts/capability-matrix/src/normalize-griffe.ts +++ b/scripts/capability-matrix/src/normalize-griffe.ts @@ -72,6 +72,6 @@ function qual(classStack: string[], name: string): string { function emit(symbols: ParsedSymbol[], ig: Ignore | null, sym: ParsedSymbol, lineno?: number): void { if (ig && sym.file && ig.ignores(sym.file)) return; - if (lineno !== undefined) sym.line = lineno; - symbols.push(sym); + // Griffe inherits Python ast.lineno which is already 1-based — no +1 needed (unlike TS/Swift). + symbols.push(lineno !== undefined ? { ...sym, line: lineno } : sym); } diff --git a/scripts/capability-matrix/src/normalize-symbolgraph.ts b/scripts/capability-matrix/src/normalize-symbolgraph.ts index b57cd3e..4b2d644 100644 --- a/scripts/capability-matrix/src/normalize-symbolgraph.ts +++ b/scripts/capability-matrix/src/normalize-symbolgraph.ts @@ -41,15 +41,15 @@ export function normalizeSymbolGraph( const kind = KIND_MAP[sym.kind.identifier]; if (kind === undefined) continue; - const sym_: ParsedSymbol = { + const parsed: ParsedSymbol = { name: qualifiedName(sym.pathComponents), kind, file: resolveFile(sym.location?.uri, sdkRoot), }; if (sym.location?.position !== undefined) { - sym_.line = sym.location.position.line + 1; + parsed.line = sym.location.position.line + 1; } - result.push(sym_); + result.push(parsed); } return { symbols: result }; diff --git a/scripts/capability-matrix/test/api-check.test.ts b/scripts/capability-matrix/test/api-check.test.ts index 0248b3c..41f180b 100644 --- a/scripts/capability-matrix/test/api-check.test.ts +++ b/scripts/capability-matrix/test/api-check.test.ts @@ -61,6 +61,14 @@ describe("checkNewSymbols", () => { const result = checkNewSymbols(base, pr, compliance); expect(result.uncoveredSymbols).toEqual([sym("AuthClient.foo"), sym("AuthClient.bar")]); }); + + it("preserves file and line through to uncoveredSymbols", () => { + const withLocation: ParsedSymbol = { name: "AuthClient.signInWithPasskey", kind: "method", file: "src/auth.ts", line: 42 }; + const result = checkNewSymbols([], [withLocation], compliance); + expect(result.uncoveredSymbols).toHaveLength(1); + expect(result.uncoveredSymbols[0].file).toBe("src/auth.ts"); + expect(result.uncoveredSymbols[0].line).toBe(42); + }); }); describe("checkNewSymbols — removed registered symbols", () => { diff --git a/scripts/dart_symbol_extractor/lib/src/extractor.dart b/scripts/dart_symbol_extractor/lib/src/extractor.dart index 1961036..b848331 100644 --- a/scripts/dart_symbol_extractor/lib/src/extractor.dart +++ b/scripts/dart_symbol_extractor/lib/src/extractor.dart @@ -40,7 +40,7 @@ void _visitTopLevel( LineInfo lineInfo, List out, ) { - final line = lineInfo.getLocation(declaration.offset).lineNumber; + final line = lineInfo.getLocation(declaration.firstTokenAfterCommentAndMetadata.offset).lineNumber; switch (declaration) { // Class-like containers expose their name and members identically. Unnamed // extensions (name == null) fall through, as they have no qualifiable @@ -96,7 +96,7 @@ void _emitContainer( ); for (final member in members) { - final memberLine = lineInfo.getLocation(member.offset).lineNumber; + final memberLine = lineInfo.getLocation(member.firstTokenAfterCommentAndMetadata.offset).lineNumber; if (member is MethodDeclaration) { final name = member.name.lexeme; if (_isPrivate(name)) continue; diff --git a/scripts/dart_symbol_extractor/test/extractor_test.dart b/scripts/dart_symbol_extractor/test/extractor_test.dart index 3bee005..9f792a0 100644 --- a/scripts/dart_symbol_extractor/test/extractor_test.dart +++ b/scripts/dart_symbol_extractor/test/extractor_test.dart @@ -123,6 +123,28 @@ extension type UserId(String value) { expect(names, containsAll(['UserId', 'UserId.isValid'])); }); + test('reports line of declaration keyword, not annotation, for annotated members', () { + const source = ''' +class MyClient { + @override + void signIn() {} + @Deprecated('use signOut2') + void signOut() {} +} +'''; + final symbols = extractFromSource(source, 'lib/client.dart'); + // `void signIn()` is on the line after @override (not on @override's line). + final signInLine = _byName(symbols, 'MyClient.signIn').line!; + final signOutLine = _byName(symbols, 'MyClient.signOut').line!; + // signOut's declaration line must be strictly after signIn's annotation line, + // confirming firstTokenAfterCommentAndMetadata skips metadata. + expect(signOutLine, greaterThan(signInLine + 1), + reason: 'signOut should start after signIn and its @Deprecated annotation'); + // Neither should land on line 1 (the class keyword line) or line 2 (@override). + expect(signInLine, greaterThan(2)); + expect(signOutLine, greaterThan(signInLine)); + }); + test('captures top-level functions, variables and typedefs', () { const source = ''' String greet(String name) => 'hi'; From 2b12fd93d9c571d604a2e24250dded49e6dd21dd Mon Sep 17 00:00:00 2001 From: Guilherme Souza Date: Thu, 25 Jun 2026 14:26:04 -0300 Subject: [PATCH 3/3] chore(dart): apply dart format --- .../lib/src/extractor.dart | 18 +++++++++++++----- .../lib/src/parsed_symbol.dart | 3 ++- .../test/extractor_test.dart | 16 +++++++++++----- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/scripts/dart_symbol_extractor/lib/src/extractor.dart b/scripts/dart_symbol_extractor/lib/src/extractor.dart index b848331..4de8e5d 100644 --- a/scripts/dart_symbol_extractor/lib/src/extractor.dart +++ b/scripts/dart_symbol_extractor/lib/src/extractor.dart @@ -40,7 +40,9 @@ void _visitTopLevel( LineInfo lineInfo, List out, ) { - final line = lineInfo.getLocation(declaration.firstTokenAfterCommentAndMetadata.offset).lineNumber; + final line = lineInfo + .getLocation(declaration.firstTokenAfterCommentAndMetadata.offset) + .lineNumber; switch (declaration) { // Class-like containers expose their name and members identically. Unnamed // extensions (name == null) fall through, as they have no qualifiable @@ -71,8 +73,8 @@ void _visitTopLevel( } } -void _emit( - String name, SymbolKind kind, String relPath, int line, List out) { +void _emit(String name, SymbolKind kind, String relPath, int line, + List out) { if (_isPrivate(name)) return; out.add(ParsedSymbol(name: name, kind: kind, file: relPath, line: line)); } @@ -96,7 +98,9 @@ void _emitContainer( ); for (final member in members) { - final memberLine = lineInfo.getLocation(member.firstTokenAfterCommentAndMetadata.offset).lineNumber; + final memberLine = lineInfo + .getLocation(member.firstTokenAfterCommentAndMetadata.offset) + .lineNumber; if (member is MethodDeclaration) { final name = member.name.lexeme; if (_isPrivate(name)) continue; @@ -104,7 +108,11 @@ void _emitContainer( ? SymbolKind.property : SymbolKind.method; out.add( - ParsedSymbol(name: '$containerName.$name', kind: kind, file: relPath, line: memberLine), + ParsedSymbol( + name: '$containerName.$name', + kind: kind, + file: relPath, + line: memberLine), ); } else if (member is FieldDeclaration) { for (final field in member.fields.variables) { diff --git a/scripts/dart_symbol_extractor/lib/src/parsed_symbol.dart b/scripts/dart_symbol_extractor/lib/src/parsed_symbol.dart index 3e70e52..b5f28bc 100644 --- a/scripts/dart_symbol_extractor/lib/src/parsed_symbol.dart +++ b/scripts/dart_symbol_extractor/lib/src/parsed_symbol.dart @@ -6,7 +6,8 @@ enum SymbolKind { classKind, method, property, function, variable } class ParsedSymbol { - ParsedSymbol({required this.name, required this.kind, required this.file, this.line}); + ParsedSymbol( + {required this.name, required this.kind, required this.file, this.line}); final String name; final SymbolKind kind; diff --git a/scripts/dart_symbol_extractor/test/extractor_test.dart b/scripts/dart_symbol_extractor/test/extractor_test.dart index 9f792a0..4f8946d 100644 --- a/scripts/dart_symbol_extractor/test/extractor_test.dart +++ b/scripts/dart_symbol_extractor/test/extractor_test.dart @@ -45,8 +45,10 @@ class SupabaseClient { ); // All symbols should have a non-null, positive line number. for (final sym in symbols) { - expect(sym.line, isNotNull, reason: '${sym.name} should have a line number'); - expect(sym.line, greaterThan(0), reason: '${sym.name} line should be > 0'); + expect(sym.line, isNotNull, + reason: '${sym.name} should have a line number'); + expect(sym.line, greaterThan(0), + reason: '${sym.name} line should be > 0'); } // Spot-check that SupabaseClient is on line 1 (first non-empty line) and // signIn follows the setter on line 7. @@ -123,7 +125,9 @@ extension type UserId(String value) { expect(names, containsAll(['UserId', 'UserId.isValid'])); }); - test('reports line of declaration keyword, not annotation, for annotated members', () { + test( + 'reports line of declaration keyword, not annotation, for annotated members', + () { const source = ''' class MyClient { @override @@ -139,7 +143,8 @@ class MyClient { // signOut's declaration line must be strictly after signIn's annotation line, // confirming firstTokenAfterCommentAndMetadata skips metadata. expect(signOutLine, greaterThan(signInLine + 1), - reason: 'signOut should start after signIn and its @Deprecated annotation'); + reason: + 'signOut should start after signIn and its @Deprecated annotation'); // Neither should land on line 1 (the class keyword line) or line 2 (@override). expect(signInLine, greaterThan(2)); expect(signOutLine, greaterThan(signInLine)); @@ -208,7 +213,8 @@ typedef Json = Map; test('includes non-null line numbers for all symbols', () { final symbols = parseDartProject('test/fixtures/sample_project'); for (final sym in symbols) { - expect(sym.line, isNotNull, reason: '${sym.name} should have a line number'); + expect(sym.line, isNotNull, + reason: '${sym.name} should have a line number'); expect(sym.line, greaterThan(0)); } });