generator: add some sanity to the language version logic#530
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces SDK version validation for target packages by adding a checkSdkVersion utility and a deriveLanguageVersion helper. These tools verify that a package's pubspec.yaml contains an SDK constraint compatible with the generator's requirements. The sdkVersion constant was renamed to dartLangugeVersion and integrated into the CLI and configuration logic. Review feedback consistently points out a spelling error in the newly introduced dartLangugeVersion identifier, recommending it be corrected to dartLanguageVersion throughout the code, documentation, and error strings.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements SDK version validation to ensure target packages meet the minimum required Dart language version of 3.10.0. It renames the existing version constant to dartLanguageVersion and introduces a checkSdkVersion function to verify pubspec.yaml constraints. Feedback highlights the need to handle VersionUnion types during version derivation, recommends addressing the silent failure when a pubspec.yaml file is missing, and suggests wrapping I/O and parsing logic in try-catch blocks to prevent unhandled exceptions from crashing the tool.
| /// constraint that aligns with [dartLanguageVersion]. | ||
| void checkSdkVersion(String targetPackagePath) { | ||
| final pubspecFile = File(p.join(targetPackagePath, 'pubspec.yaml')); | ||
| if (!pubspecFile.existsSync()) return; |
There was a problem hiding this comment.
The function returns silently if pubspec.yaml is missing. While this might be intentional to allow running the tool outside of a package, it bypasses the "sanity check" entirely. If the tool is intended to be used within a Dart package context, it might be better to throw an exception or at least log a warning when the file is missing, especially since the error message on line 41 suggests that a pubspec.yaml is expected.
| ); | ||
| }); | ||
|
|
||
| test('language version derived from SDK constraint', () { |
There was a problem hiding this comment.
Don't know how much it's worth it, but you can create a fake pubspec.yaml and then test checkSdkVersion directly. Then, you don't need to test deriveLanguageVersion and can just make it private. You can also test that SdkVersionExceptions are thrown that way.
There was a problem hiding this comment.
Wait...explain this? Just for testing?
There was a problem hiding this comment.
Yup, just so we're testing the feature actually being used in the generators instead of a subfunction (checkSdkVersion vs deriveLanguageVersion).
…, webdev, webdriver Revisions updated by `dart tools/rev_sdk_deps.dart`. ai (https://github.com/dart-lang/ai/compare/3d3b7fd..7270c43): 7270c43 Wed Apr 29 01:14:51 2026 +0530 Sourav-Sonkar fix: graceful shutdown of orphaned test apps on windows (dart-lang/ai#438) 07af327 Tue Apr 28 11:11:31 2026 -0700 Jacob MacDonald Update dependencies (dart-lang/ai#453) ff1866d Wed Apr 29 00:13:33 2026 +0800 Goweii fix: Add empty properties object to tool input schemas to resolve Claude 400 validation error (dart-lang/ai#444) 8ce7450 Thu Apr 16 11:03:55 2026 -0700 Jacob MacDonald add gemini code assist configuration for reviews (dart-lang/ai#443) 899e562 Tue Apr 14 11:38:29 2026 -0700 Jacob MacDonald Merge cherry-pick branch into main (dart-lang/ai#442) 5eae0ae Tue Apr 14 18:27:49 2026 +0000 Jake Macdonald Merge remote-tracking branch 'origin/cherry-pick' into merge-cherry-pick 1e69aa8 Tue Apr 14 11:20:43 2026 -0700 Jacob MacDonald re-implement most of `#440` for the current stable release (dart-lang/ai#441) core (https://github.com/dart-lang/core/compare/347df4b..be0b1531): be0b1531 Wed Apr 29 16:50:55 2026 -0700 Nate Bosch Revert "Completely new cross-platform API introduced" (#957) a23519da Mon Apr 20 15:57:58 2026 -0700 Nate Bosch Support variable duration in RestartableTimer (dart-lang/core#949) 0def20c4 Thu Apr 2 08:40:37 2026 +0200 Lasse R.H. Nielsen Completely new cross-platform API introduced dartdoc (https://github.com/dart-lang/dartdoc/compare/c01cf53..2e30b8e): 2e30b8e3 Tue Apr 21 15:09:21 2026 -0700 Kevin Moore Improve @canonicalFor validation with close-match suggestions (dart-lang/dartdoc#4234) fbc326ea Tue Apr 14 09:18:26 2026 -0700 Kevin Moore Fix dartdoc canonicalization for explicit getters and prioritize @canonicalFor (dart-lang/dartdoc#4233) ecosystem (https://github.com/dart-lang/ecosystem/compare/3bd43d6..ed4e053): ed4e053 Wed Apr 15 08:30:59 2026 -0700 Kevin Moore fix(workflows): prevent duplicate PR comments in post_summaries.yaml (dart-lang/ecosystem#412) http (https://github.com/dart-lang/http/compare/fa2d2c2..2c84f24): 2c84f24 Mon Apr 27 14:45:17 2026 -0700 Brian Quinlan doc: s/dart:html/package:web/g (dart-lang/http#1911) i18n (https://github.com/dart-lang/i18n/compare/de7e11b..2ae32fd): 2ae32fdd Wed Apr 22 02:20:51 2026 -0700 Googler No public description 1eaf8a2e Sun Mar 1 06:51:04 2026 +0000 dependabot[bot] Bump the github-actions group with 2 updates (dart-lang/i18n#1053) shelf (https://github.com/dart-lang/shelf/compare/c4b94d3..cc6b57d): cc6b57d Wed Apr 29 08:59:40 2026 -0700 Kevin Moore chore: update shelf_io test to not crash on certificate error (dart-lang/shelf#522) da9f1b1 Thu Apr 23 14:43:00 2026 -0400 Mouad Debbar Fix multi-header handling in shelf_proxy (dart-lang/shelf#519) a6693a9 Wed Apr 22 23:15:09 2026 -0700 Kevin Moore feat: Add HTTP/1.1 compliance testing package and baseline for shelf (dart-lang/shelf#518) test (https://github.com/dart-lang/test/compare/8bbb847..d5da922): d5da9229 Mon Apr 27 17:32:33 2026 -0700 Jacob MacDonald Expose the backend APIs used by flutter_test from test_api (dart-lang/test#2635) c8c76f45 Mon Apr 27 13:38:25 2026 -0700 Nate Bosch Prepare to publish (dart-lang/test#2634) web (https://github.com/dart-lang/web/compare/15599ee..7c908b1): 7c908b1 Tue Apr 28 17:16:57 2026 -0700 Kevin Moore test(js_interop_gen): add integration tests (dart-lang/web#541) 5ed9bed Tue Apr 28 17:04:41 2026 -0700 Kevin Moore js_interop_gen: minimize the number of `@docImport` comments we create (dart-lang/web#540) 2c47e36 Mon Apr 27 20:55:43 2026 -0700 Kevin Moore feat: generate descriptive documentation for union typedefs (dart-lang/web#534) db91caf Mon Apr 27 15:15:30 2026 -0700 Kevin Moore chore: write BCD version to README in web_generator (dart-lang/web#537) 5e7285f Fri Apr 24 12:09:10 2026 -0700 Kevin Moore chore: update node invocation to use the source maps we already generate (dart-lang/web#538) 3a7e37a Sat Apr 18 19:31:20 2026 -0700 Kevin Moore Stabilize JS Interop CI and decouple webref dependencies (dart-lang/web#532) d7daf7d Tue Apr 14 12:23:21 2026 -0700 Kevin Moore generator: add some sanity to the language version logic (dart-lang/web#530) b026317 Mon Apr 13 13:01:37 2026 -0700 Kevin Moore generator: be more robust against NPM timing issues (dart-lang/web#528) webdev (https://github.com/dart-lang/webdev/compare/2947c78..d33d270): d33d2704 Thu Apr 23 16:00:22 2026 -0400 Jessy Yameogo [dwds] Sync state with sdk/g3 and restore static analysis for fixtures (dart-lang/webdev#2824) e43744b2 Fri Apr 17 16:55:24 2026 -0400 Jessy Yameogo [dwds] Fix ping event serialization by introducing PingRequest (dart-lang/webdev#2821) 1063eb00 Fri Apr 17 10:28:47 2026 -0700 MarkZ Creating experimental flag for web hot reload in webdev and splitting tests between DDC module systems (dart-lang/webdev#2807) webdriver (https://github.com/google/webdriver.dart/compare/26326d3..3a711eb): 3a711eb Mon Apr 27 10:09:31 2026 -0700 Kevin Moore Update CI badge to point to master branch (google/webdriver.dart#343) R=bquinlan@google.com Change-Id: Ie746dfd8f9637db46075b252d3290925de11b382 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/499663 Reviewed-by: Brian Quinlan <bquinlan@google.com> Commit-Queue: Nate Bosch <nbosch@google.com>
2 participants
No description provided.