Skip to content

clean: replace regular expressions with structured parsing #66

Description

@walterjgsp

Summary

Regular expressions are used in four layers of this repository: the Go parsers of each language's helper tool, one Python script, the release job, and the CI checks. Several of them parse something that has a structured form (JUnit XML, a WIT or TOML file, Please's coverage JSON, the .lcov format the shared package already parses), or edit a file that a release depends on. A regex there fails silently when the format drifts, or matches more than intended. This issue lists every use, ranks the risk, and proposes a structured replacement for each, to be done as small PRs.

It came out of a bug we hit: a regex-based edit of test code silently dropped arguments, and the release job rewrites tools/BUILD with re.sub, which does nothing if the file's format changes.

Inventory

# Where What it does Risk Replacement
1 release.yml (all language branches) re.sub rewrites _VERSION and the hashes of tools/BUILD; the hashes pattern replaces every hashes block it finds (Kotlin needed a lazy cross-block pattern per tool) High: silent no-op, stale pins in a published release Done: scripts/update_release_pins.py (#63, synced by #64 for swift and #65 for kotlin)
2 Kotlin tools/please_kotlin/testrunner/junit.go One long case-insensitive pattern over the JUnit launcher's console output High: the runner already passes --reports-dir, so the launcher writes JUnit XML that is ignored Read the XML with encoding/xml
3 WIT tools/please_wit/generate/generate.go (worldRegex, packageRegex); Kotlin tools/please_kotlin_wasm/compile.go (funcRegex, packageRegex, ifaceRegex, resourceRegex) Regexes for package, world, interface, func(...) over WIT source High: a real parser exists (tools/please_wit/ast) and generate.go already calls ast.ParsePath elsewhere Use ast. Kotlin cannot import it from the wit branch, so move it to tools/common on main. The Kotlin package and class lines can use a plain line scanner
4 CI: ci-main.yml branch guard; the coverage smoke steps of each language grep -E on file lists, on Please's console coverage table, and on .lcov text Medium: these broke when the console format differed case globs for the guard; jq on plz-out/log/coverage.json for line coverage; a small lcovcheck command on the shared tools/common/lcov parser for the .lcov assertions (statuses, functions, branches)
5 scripts/generate_release_notes.py Cuts a changelog section out with re.search(rf"## \[v?{re.escape(version)}\]...", re.DOTALL); a list-marker regex for unwrapping Medium: multi-line DOTALL matching over markdown A line scanner: find the ## [version] heading, collect lines to the next ## [; string methods for list markers
6 Rust tools/please_rust/download/metadata.go Regexes over Cargo.toml for edition and [lib] path Medium: a multi-line value or inline table slips past them A TOML parser (adds a pinned dependency such as BurntSushi/toml)
7 Swift tools/please_swift/testrunner/runner.go About ten patterns over human-readable output (Unicode symbols, ANSI codes) Medium: only a fallback, the JSON event stream is parsed first Rely on the event stream; drop or minimise the fallback
8 Rust tools/please_rust/testrunner/junit.go Two patterns over cargo test text Low to medium: stable libtest has no structured output (JSON needs a nightly flag) strings.Cut / strings.Fields on the documented test x ... ok lines
9 Rust tools/please_rust/compile/deps.go -[0-9a-f]{16}$ to strip a crate hash suffix Low A small function: length, the -, hex check
10 TypeScript tools/please_ts/testrunner/vitest.go regexp.QuoteMeta to build exact-match alias regexes for the generated Vite config Low, but needless Generate a small Vite plugin that looks names up in an exact map

Not a problem: the Starlark rules only use startswith, main has no regex in Go, and the ANSI-strip pattern in Swift is a standard idiom (keep it if the fallback stays).

Plan

Order by risk, one small PR per item on the branch that owns the code:

  1. Release pins (docs: replace rust_rules_plugin_plan with comprehensive documentation #1) done; remaining: a dry_run release dispatch to validate the new step end to end.
  2. Kotlin: read the JUnit XML (feat: Add hermetic TypeScript and Deno build rules (ts_module, ts_library, ts_binary, ts_test) #2).
  3. CI assertions (feat: Implement hermetic rust_toolchain rule #4): lcovcheck, jq, case globs.
  4. WIT and Kotlin wasm onto ast (feat: Implement hermetic rust_toolchain rule #3).
  5. Release notes scanner (feat: Add test coverage support for Rust rules (plz cover) #5), Vitest plugin (feat(kotlin): support custom test runners in kotlin_test #10).
  6. The small Rust and Swift ones (feat(rust): implement code coverage support for Rust rules (#5) #7, feat: Implement hermetic kotlin_toolchain and Please Kotlin build rules #8, feat: Implement hermetic swift_toolchain and Please Swift build rules #9), and TOML (ci: Automate the release lifecycle with GitHub Actions #6) if the dependency is acceptable.

Rules for the replacements

  • Prefer an existing structured output or parser over text parsing; where none exists, a small scanner (strings.Cut, Fields) with tests beats a pattern.
  • Edits of files that a release or build depends on are done by structure, read back and verified, and fail loudly on anything unexpected.
  • Each replacement keeps the old tests' cases (the fixtures are the specification) and adds the cases the regex got wrong.

Acceptance criteria

Activity

  1. added
    cleanCleaning the source code and potential technical debts
    on Oct 4, 2026
  2. walterjgsp commented on Oct 4, 2026

    @walterjgsp
    ContributorAuthor

    Dry-run releases of the new pin step ran on both variants of release.yml and succeeded (run 37202601293 on rust as 0.6.1, run 37202603083 on kotlin as 0.4.1; nothing was committed, tagged or published).

    • rust (one tool): only _VERSION and the four hash lines of please_rust changed.
    • kotlin (two tools): _VERSION, and the four hashes of each of please_kotlin and please_kotlin_wasm changed, with different values per tool, and nothing else in the file.

    That completes item 1 and its acceptance criterion (dry_run release on rust and kotlin). Remaining items 2 to 10 are untouched.

  3. walterjgsp commented on Oct 4, 2026

    @walterjgsp
    ContributorAuthor

    Item 2 (Kotlin JUnit) is in #67. Reading the code before changing it showed the situation was a little different from the inventory: the runner already preferred the launcher's XML, but it copied only the first .xml in the reports directory, while the launcher writes one report per engine (Jupiter, Vintage, Platform Suite). Results of any other engine were silently lost, <error> and <skipped> were not modelled, and the regex fallback never matched the launcher's real tree-style output (its tests only used synthetic lines). #67 reads every report with encoding/xml, recounts from the test cases, and drops the regex. Checked end to end through Please with a passing, a failing and a skipped test.

  4. walterjgsp commented on Oct 4, 2026

    @walterjgsp
    ContributorAuthor

    Item 4 (CI assertions) is done:

    Items done so far: 1 (release pins, #63 to #65 and the dry runs), 2 (Kotlin JUnit XML, #67), 4 (CI assertions). Next in the plan: 3 (WIT and Kotlin wasm onto ast), 5 (release notes scanner, Vitest plugin), then 6 to 9.

  5. walterjgsp commented on Oct 4, 2026

    @walterjgsp
    ContributorAuthor

    Item 3 (WIT and Kotlin wasm regexes) is done:

    Left from #66: 5 (release notes line scanner, Vitest alias plugin), 6 to 9 (Swift fallback, Rust cargo-test parser, deps.go, TOML). DetectPackage/hasClassInSources in kotlin_wasm use regexes on Kotlin source (fits with the Kotlin items).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    cleanCleaning the source code and potential technical debts

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions