-
Notifications
You must be signed in to change notification settings - Fork 15
fix(tracer): fix BSD-incompatible sed injection at remaining jvm/native call sites #2047
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.
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
base: fix/issue-1911-stale-doc-comment-roleclassificationnode
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| /** | ||
| * Regression coverage for #1913: the sed-injection helpers in | ||
| * tracer-common.sh must produce output that is byte-identical on GNU sed | ||
| * (Linux CI) and BSD sed (macOS dev boxes), which — unlike GNU — rejects the | ||
| * single-line `a\text` / `i\text` shortcut and requires the text on the line | ||
| * after the backslash. | ||
| * | ||
| * These tests exercise the shared helpers directly with bash + sed (whatever | ||
| * sed the host provides), independent of any per-language compiler toolchain | ||
| * (javac/kotlinc/scalac/groovyc/dotnet/dart/zig), so they run identically in | ||
| * every environment regardless of which toolchains happen to be installed. | ||
| */ | ||
| import { execFileSync } from 'node:child_process'; | ||
| import fs from 'node:fs'; | ||
| import os from 'node:os'; | ||
| import path from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
| import { afterEach, beforeEach, describe, expect, it } from 'vitest'; | ||
|
|
||
| const __dirname = path.dirname(fileURLToPath(import.meta.url)); | ||
| const TRACER_DIR = __dirname; | ||
| const TRACER_COMMON = path.join(TRACER_DIR, 'tracer-common.sh'); | ||
|
|
||
| function runHelper(fnCall: string, file: string): string { | ||
| execFileSync('bash', ['-c', `source "${TRACER_COMMON}"; ${fnCall}`]); | ||
|
Check failure on line 25 in tests/benchmarks/resolution/tracer/tracer-common.test.ts
|
||
| return fs.readFileSync(file, 'utf8'); | ||
| } | ||
|
|
||
| describe('tracer-common.sh sed-injection helpers (#1913)', () => { | ||
| let tmpDir: string; | ||
|
|
||
| beforeEach(() => { | ||
| tmpDir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'tracer-common-test-'))); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| fs.rmSync(tmpDir, { recursive: true, force: true }); | ||
| }); | ||
|
|
||
| it('sedi_insert_before inserts text directly before the matching line (native-tracer.sh csharp RunWithValidation site)', () => { | ||
| const file = path.join(tmpDir, 'Program.cs'); | ||
| fs.writeFileSync( | ||
| file, | ||
| [ | ||
| 'class Program {', | ||
| ' public static void Main() {', | ||
| ' CallTracer.Dump();', | ||
| ' }', | ||
| '}', | ||
| '', | ||
| ].join('\n'), | ||
| ); | ||
|
|
||
| const out = runHelper( | ||
| `sedi_insert_before '/CallTracer.Dump/' ' RunWithValidation();' "${file}"`, | ||
| file, | ||
| ); | ||
|
|
||
| expect(out).toContain(' RunWithValidation();\n CallTracer.Dump();'); | ||
|
Check failure on line 59 in tests/benchmarks/resolution/tracer/tracer-common.test.ts
|
||
| }); | ||
|
|
||
| it('sedi_insert_before_end inserts before the matching brace within a range (jvm/native dump-injection sites)', () => { | ||
| const file = path.join(tmpDir, 'main.dart'); | ||
| fs.writeFileSync(file, ['void main() {', ' print(1);', '}', ''].join('\n')); | ||
|
|
||
| const out = runHelper( | ||
| `sedi_insert_before_end '/^void main/' '/^\\}/' '/^\\}/' ' CallTracer.instance.dump();' "${file}"`, | ||
| file, | ||
| ); | ||
|
|
||
| expect(out.split('\n')).toEqual([ | ||
| 'void main() {', | ||
| ' print(1);', | ||
| ' CallTracer.instance.dump();', | ||
| '}', | ||
| '', | ||
| ]); | ||
| }); | ||
|
|
||
| it('sedi_insert_before_end honors distinct range-end and insert-line patterns (jvm-tracer.sh java dump site)', () => { | ||
| // java's dump site scopes the range with a loose /\}/ end-address but | ||
| // only inserts before a line that is *purely* a closing brace, so the | ||
| // range and insertion patterns must be tracked independently. | ||
| const file = path.join(tmpDir, 'Main.java'); | ||
| fs.writeFileSync( | ||
| file, | ||
| [ | ||
| 'public class Main {', | ||
| ' public static void main(String[] args) {', | ||
| ' foo();', | ||
| ' }', | ||
| '}', | ||
| '', | ||
| ].join('\n'), | ||
| ); | ||
|
|
||
| const out = runHelper( | ||
| `sedi_insert_before_end '/public static void main/' '/\\}/' '/^[[:space:]]*\\}/' ' CallTracer.dump();' "${file}"`, | ||
| file, | ||
| ); | ||
|
|
||
| expect(out).toContain(' foo();\n CallTracer.dump();\n }\n}'); | ||
| }); | ||
|
|
||
| it('sedi_append_unless appends after matching lines except negated ones, including a space before the brace (jvm-tracer.sh java/groovy per-method sites)', () => { | ||
| const file = path.join(tmpDir, 'BaseService.java'); | ||
| fs.writeFileSync( | ||
| file, | ||
| [ | ||
| 'public abstract class BaseService {', | ||
| '', | ||
| ' protected void log(String message) {', | ||
| ' System.out.println(message);', | ||
| ' }', | ||
| '}', | ||
| '', | ||
| ].join('\n'), | ||
| ); | ||
|
|
||
| const out = runHelper( | ||
| `sedi_append_unless '/\\)[[:space:]]*\\{$/' '/class |interface /' ' CallTracer.traceCall();' "${file}"`, | ||
| file, | ||
| ); | ||
|
|
||
| // The method signature line (ending in ") {") gets the call appended... | ||
| expect(out).toContain( | ||
| ' protected void log(String message) {\n CallTracer.traceCall();', | ||
| ); | ||
| // ...but the class declaration line (also ending in "{") is excluded. | ||
| expect(out.split('\n')[0]).toBe('public abstract class BaseService {'); | ||
| }); | ||
|
|
||
| it('rejects a regression back to the GNU-only single-line a\\/i\\ shortcut in the tracer scripts', () => { | ||
| // Every dump()/traceCall() injection now goes through the shared helpers | ||
| // above, so the raw sed scripts embedded directly in the language | ||
| // tracers should contain zero remaining single-line "i\text"/"a\text" | ||
| // occurrences (the exact defect #1913 fixed) — this guards against | ||
| // future injection sites reintroducing the GNU-only shortcut. | ||
| const guardedFiles = ['jvm-tracer.sh', 'native-tracer.sh', 'go-tracer.sh']; | ||
|
|
||
| for (const name of guardedFiles) { | ||
| const code = fs | ||
| .readFileSync(path.join(TRACER_DIR, name), 'utf8') | ||
| .split('\n') | ||
| // Drop comment lines so prose that *describes* the banned form (as | ||
| // this very fix's changelog comments do) isn't mistaken for it. | ||
| .filter((line) => !line.trim().startsWith('#')) | ||
| .join('\n'); | ||
| // \S excludes the newline that must immediately follow a portable | ||
| // "i\" / "a\" — so this only matches the banned single-line form | ||
| // where real text follows the backslash on the same line. | ||
| const offender = code.match(/[ai]\\\S/); | ||
| expect( | ||
| offender, | ||
| `${name} contains a GNU-only single-line a\\/i\\ form: ${offender}`, | ||
| ).toBeNull(); | ||
| } | ||
| }); | ||
| }); | ||
|
Comment on lines
+139
to
+159
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The guard intentionally skips Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
${pattern} i\applies to every line that matches the pattern, not only the first. The current sole call-site happens to target a unique string (/CallTracer.Dump/), so the mismatch is harmless today — but a future caller that expects first-only semantics would silently get multiple insertions. The comment should say "every" (or restrict the implementation to a0,/pat/-scoped address for truly first-only behaviour).Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!