Skip to content

linters(dart): remove dart shim, bump to 3.10.8#1114

Open
AndrewDongminYoo wants to merge 3 commits intotrunk-io:mainfrom
AndrewDongminYoo:main
Open

linters(dart): remove dart shim, bump to 3.10.8#1114
AndrewDongminYoo wants to merge 3 commits intotrunk-io:mainfrom
AndrewDongminYoo:main

Conversation

@AndrewDongminYoo
Copy link
Contributor

@AndrewDongminYoo AndrewDongminYoo commented Feb 4, 2026

Summary

  • Bump known_good_version to Dart 3.10.8 and wire run_from for format/fix commands to prevent per-file directory traversal overhead
  • Set fix to opt-in (enabled: false), keep analyze as the default diagnostic command aligning with the pattern used by ruff, sqlfluff, and terraform
  • Remove tools.definitions entirely and flatten download/environment into lint.definitions, eliminating the dart shim in .trunk/tools/ to prevent it from shadowing the project-local Flutter/Dart SDK binary (same structure as clippy and gofmt)
  • Reorder environment.PATH to ["${env.PATH}", "${linter}/bin"] so that a project-local dart (e.g. managed by Flutter) is preferred when available, with trunk's downloaded SDK as fallback

Changes in detail

known_good_version 3.9.2 → 3.10.8

Bumps the default Dart SDK version trunk downloads for linting.
This version is used as a fallback when no system dart is available on PATH (see the PATH reorder section below).

suggest_if: files_presentconfig_present

Avoids auto-suggesting the dart linter in projects that have .dart files but no
analysis_options.yaml.
Reduces noise for projects that don't actively use the Dart analyzer.

run_from on format and fix

dart format and dart fix are slow when trunk moves into each file's directory individually.
Setting run_from: ${root_or_parent_with(analysis_options.yaml)} batches execution from the project root (or nearest ancestor with analysis_options.yaml), matching how the Dart toolchain expects to be invoked.

fixenabled: false

analyze is the only command that produces diagnostic output.
Other linters in this repo (ruff, sqlfluff, terraform) follow the same convention: the diagnostic command stays enabled by default, and mutating commands like fix are opt-in.
Reversing this would mean users who enable the dart linter receive no lint diagnostics at all.

Remove tools.definitions / shim

When tools.definitions declares shims: [dart], trunk exposes a dart binary under .trunk/tools/.
If .trunk/tools is on PATH (as trunk adds it), this silently shadows the project's own dart binary (typically managed by Flutter).
clippy and gofmt avoid this by placing download directly inside lint.definitions with no separate tool block.
This PR applies the same pattern.

Prefer system dart on PATH

Previously environment.PATH was set to ["${linter}/bin"], which meant trunk's downloaded dart was the only binary available when running lint commands.
This PR changes it to ["${env.PATH}", "${linter}/bin"]: the system PATH is checked first, so a Flutter-managed dart binary is used when present.
Trunk's downloaded SDK remains as a fallback for environments without one (e.g. bare CI).
The same pattern is used by buf, trivy, and clippy in this repo.

Test plan

  • dart linter enables and runs analyze by default
  • dart format executes without per-file directory errors
  • dart fix does not run unless explicitly enabled (trunk check enable dart → verify no fix action in output)
  • No dart binary appears under .trunk/tools/ after enabling the linter
  • Existing Flutter project with a pinned Dart SDK: project-local dart binary is not overridden on PATH
  • Environment without Flutter/dart on PATH: trunk's downloaded SDK is used as fallback and linting succeeds

… fix command

- Bump known_good_version to 3.10.8
- Remove dartaotruntime shim (unused in current SDK)
- Change suggest_if to config_present to avoid false suggestions
- Add run_from ${root_or_parent_with(analysis_options.yaml)} to format and fix
  commands to prevent per-file directory traversal overhead
- Set fix to enabled: false (opt-in), keep analyze enabled as the primary
  diagnostic command, aligning with the pattern used by ruff/sqlfluff/terraform
- Update test snapshots accordingly
Remove tools.definitions block entirely to prevent trunk from exposing
a dart shim in .trunk/tools/, which could shadow the project-local dart
binary (Flutter SDK). Move download and environment into lint.definitions
directly, following the same pattern as clippy and gofmt.
Reorder environment PATH to ["${env.PATH}", "${linter}/bin"] so that a
project-local dart binary (e.g. managed by Flutter) is used when available.
Falls back to trunk's downloaded SDK when no system dart exists. Same
pattern as buf, trivy, and clippy.
@AndrewDongminYoo
Copy link
Contributor Author

Hi @TylerJang27 👋

I wanted to follow up on #1098, which you kindly reviewed and merged back in October.

After using the dart plugin in production, I discovered a critical issue with my original implementation: the shims: [dart] declaration was causing .trunk/tools/dart to shadow the project's Flutter-managed dart binary (in .dart_tool directory). This meant developers would unknowingly run trunk's downloaded SDK instead of their project's pinned version, which defeats the purpose of having SDK version constraints in pubspec.yaml.

I've submitted #1114 to address this and other edge cases that emerged from real-world usage:

Key fixes:

  • Removed the dart shim entirely to prevent PATH shadowing (following the pattern used by clippy and gofmt)
  • Reordered environment.PATH to ["${env.PATH}", "${linter}/bin"] so the system dart is preferred, with trunk's SDK as fallback

Testing:
Verified on both Flutter-installed and bare CI environments. The plugin now respects the project's dart version when available, while maintaining hermetic behavior when it's not.

I apologize for not catching these issues during the initial PR.
Would appreciate your review on #1114 when you have time. Thanks again for maintaining this project!

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant