- Notifications
You must be signed in to change notification settings - Fork77
Change classes and constructors to new/primary syntax.#2233
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Package publishing
Documentation athttps://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
PR HealthChangelog Entry ❗
Changes to files need to beaccounted for in their respective changelogs. This check can be disabled by tagging the PR with Coverage |
| File | Coverage |
|---|---|
| pkgs/package_config/bin/package_config_of.dart | 💔 Not covered |
| pkgs/package_config/lib/src/errors.dart | 💔 0 % ⬇️ 100 % |
| pkgs/package_config/lib/src/package_config.dart | 💔 0 % ⬇️ 100 % |
This check fortest coverage is informational (issues shown here will not fail the PR).
This check can be disabled by tagging the PR withskip-coverage-check.
License Headers ✔️
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file// for details. All rights reserved. Use of this source code is governed by a// BSD-style license that can be found in the LICENSE file.| Files |
|---|
| no missing headers |
All source files should start with alicense header.
Unrelated files missing license headers
| Files |
|---|
| pkgs/bazel_worker/benchmark/benchmark.dart |
| pkgs/benchmark_harness/integration_test/perf_benchmark_test.dart |
| pkgs/boolean_selector/example/example.dart |
| pkgs/clock/lib/clock.dart |
| pkgs/clock/lib/src/clock.dart |
| pkgs/clock/lib/src/default.dart |
| pkgs/clock/lib/src/stopwatch.dart |
| pkgs/clock/lib/src/utils.dart |
| pkgs/clock/test/clock_test.dart |
| pkgs/clock/test/default_test.dart |
| pkgs/clock/test/stopwatch_test.dart |
| pkgs/clock/test/utils.dart |
| pkgs/coverage/lib/src/coverage_options.dart |
| pkgs/html/example/main.dart |
| pkgs/html/lib/dom.dart |
| pkgs/html/lib/dom_parsing.dart |
| pkgs/html/lib/html_escape.dart |
| pkgs/html/lib/parser.dart |
| pkgs/html/lib/src/constants.dart |
| pkgs/html/lib/src/encoding_parser.dart |
| pkgs/html/lib/src/html_input_stream.dart |
| pkgs/html/lib/src/list_proxy.dart |
| pkgs/html/lib/src/query_selector.dart |
| pkgs/html/lib/src/token.dart |
| pkgs/html/lib/src/tokenizer.dart |
| pkgs/html/lib/src/treebuilder.dart |
| pkgs/html/lib/src/utils.dart |
| pkgs/html/test/dom_test.dart |
| pkgs/html/test/parser_feature_test.dart |
| pkgs/html/test/parser_test.dart |
| pkgs/html/test/query_selector_test.dart |
| pkgs/html/test/selectors/level1_baseline_test.dart |
| pkgs/html/test/selectors/level1_lib.dart |
| pkgs/html/test/selectors/selectors.dart |
| pkgs/html/test/support.dart |
| pkgs/html/test/tokenizer_test.dart |
| pkgs/html/test/trie_test.dart |
| pkgs/html/tool/generate_trie.dart |
| pkgs/pubspec_parse/test/git_uri_test.dart |
| pkgs/stack_trace/example/example.dart |
| pkgs/watcher/test/custom_watcher_factory_test.dart |
| pkgs/yaml_edit/example/example.dart |
This check can be disabled by tagging the PR withskip-license-check.
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.
Code Review
This pull request updates the codebase to use new Dart 3 syntax, including primary constructors andinterface classes. The changes are mostly correct and align with modern Dart practices. However, I've found a few critical issues inpkgs/package_config/lib/src/errors.dart whereimplements clauses were dropped during refactoring, breaking the type hierarchy. I've also identified several places where the new syntax for factories and constructors is used incorrectly. I've provided suggestions to fix these issues.
Uh oh!
There was an error while loading.Please reload this page.
| @sealed | ||
| classSimpleInvalidLanguageVersionextendsSimpleLanguageVersion | ||
| classSimpleInvalidLanguageVersion(String source) | ||
| extendsSimpleLanguageVersion(-1,-1, source) |
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.
Without super-class arguments, this will be an in-body
new(String source):super(-1,-1, source);
Just wanted to show what it would look like with super-class arguments. (Because I probably wouldn't use a primary constructor without it.)
| ):super.value(); | ||
| PackageConfigArgumentError.from(ArgumentError error) | ||
| ):super.value(); |
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.
I initially made this a primary constructor (using super-class arguments), but since all construction needs to go through the primary constructor, and this constructor restricts the types to non-nullable to require a name and message, thefrom() constructor could not redirect to it.
(That could be an argument for preferring an in-body declaring constructor, if you want other initializing constructors on the same class, which cannot be made to forward to the primary constructor.)
mosuem commentedNov 17, 2025
(Converting to draft PR as there is no intent to land - please undo if needed) |
Uh oh!
There was an error while loading.Please reload this page.
(DO NOT LAND!)
Example of using new constructor syntax on a code-base.
I used
SuperClass(args)/SuperClass.name(args)as well. In every case where I did that, I wouldnot have used a primary constructor otherwise.