From 9f11b2fd4592c99b8ec9a0314f84fc64e51a8ff4 Mon Sep 17 00:00:00 2001 From: Robert Nystrom Date: Wed, 27 May 2026 15:56:27 -0700 Subject: [PATCH] Migrate pkg/status_file to primary constructors and new constructor syntax. Unlike my previous couple of CLs, I went ahead here and also migrated most of the classes to use primary constructors since so many of them were small and well suited for it. In the process, I found and fixed one bug in the assist to convert to a primary constructor: https://dart-review.googlesource.com/c/sdk/+/506760 So I guess this CL has already paid its way. :) Change-Id: I9b7d08e27f4ddce3f19127aaf4f14fe627b75dab Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/506743 Reviewed-by: Paul Berry Commit-Queue: Bob Nystrom Commit-Queue: Paul Berry Auto-Submit: Bob Nystrom --- pkg/status_file/analysis_options.yaml | 4 ++ .../lib/canonical_status_file.dart | 62 +++++++------------ pkg/status_file/lib/expectation.dart | 14 ++--- pkg/status_file/lib/src/disjunctive.dart | 6 +- pkg/status_file/lib/src/expression.dart | 61 +++++++----------- pkg/status_file/lib/status_file.dart | 10 +-- pkg/status_file/lib/status_file_linter.dart | 16 +---- pkg/status_file/pubspec.yaml | 2 +- .../test/status_expression_test.dart | 2 +- 9 files changed, 65 insertions(+), 112 deletions(-) diff --git a/pkg/status_file/analysis_options.yaml b/pkg/status_file/analysis_options.yaml index e306536c160..ade30ca355b 100644 --- a/pkg/status_file/analysis_options.yaml +++ b/pkg/status_file/analysis_options.yaml @@ -8,3 +8,7 @@ analyzer: errors: # Allow having TODOs in the code todo: ignore +linter: + rules: + - unnecessary_const_in_enum_constructor + - unnecessary_type_name_in_constructor diff --git a/pkg/status_file/lib/canonical_status_file.dart b/pkg/status_file/lib/canonical_status_file.dart index faaadd1cbfa..bc001e7739b 100644 --- a/pkg/status_file/lib/canonical_status_file.dart +++ b/pkg/status_file/lib/canonical_status_file.dart @@ -74,19 +74,19 @@ class StatusFile { /// Constructor for creating a new [StatusFile]. Will not create the default /// section that status files have. - StatusFile(this.path); + new(this.path); /// Reads and parses the status file at [path]. /// /// Throws a [SyntaxError] if the file could not be parsed. - StatusFile.read(this.path) { + new read(this.path) { _parse(File(path).readAsLinesSync()); } /// Parses lines of strings coming from a status file at [path]. /// /// Throws a [SyntaxError] if the file could not be parsed. - StatusFile.parse(this.path, List lines) { + new parse(this.path, List lines) { _parse(lines.map((line) => line.trim()).toList()); } @@ -305,27 +305,26 @@ class StatusFile { /// /// Contains the condition from the header that begins the section, then all of /// the entries within the section. -class StatusSection { +class StatusSection( /// The expression that determines when this section is applied. /// /// Will be [Expression.always] for paths that appear before any section /// header in the file. In that case, the section always applies. - final Expression condition; + final Expression condition, /// The one-based line number where the section appears in the file. - final int lineNumber; + final int lineNumber, + final List sectionHeaderComments, +) { /// Collection of all comment and status line entries. final List entries = []; - final List sectionHeaderComments; /// Returns true if this section should apply in the given [environment]. bool isEnabled(Environment environment) => condition.evaluate(environment); bool isEmpty() => !entries.any((entry) => entry is StatusEntry); - StatusSection(this.condition, this.lineNumber, this.sectionHeaderComments); - @override String toString() { var buffer = StringBuffer(); @@ -338,11 +337,7 @@ class StatusSection { } } -class Comment { - final String _comment; - - Comment(this._comment); - +class Comment(final String _comment) { /// Returns the issue number embedded in [comment] or `null` if there is none. int? issueNumber(String comment) { var match = _issuePattern.firstMatch(comment); @@ -351,32 +346,20 @@ class Comment { } @override - String toString() { - return _comment; - } + String toString() => _comment; } -abstract class Entry { +abstract class Entry( /// The one-based line number where the entry appears in the file. - final int lineNumber; - - Entry(this.lineNumber); -} - -class EmptyEntry extends Entry { - EmptyEntry(super.lineNumber); + final int lineNumber, +); +class EmptyEntry(super.lineNumber) extends Entry { @override - String toString() { - return ""; - } + String toString() => ''; } -class CommentEntry extends Entry { - final Comment comment; - - CommentEntry(super.lineNumber, this.comment); - +class CommentEntry(super.lineNumber, final Comment comment) extends Entry { @override String toString() { return comment.toString(); @@ -384,13 +367,12 @@ class CommentEntry extends Entry { } /// Describes the test status of the file or files at a given path. -class StatusEntry extends Entry { - final String path; - final List expectations; - final Comment? comment; - - StatusEntry(this.path, super.lineNumber, this.expectations, this.comment); - +class StatusEntry( + final String path, + super.lineNumber, + final List expectations, + final Comment? comment, +) extends Entry { @override String toString() { return comment == null diff --git a/pkg/status_file/lib/expectation.dart b/pkg/status_file/lib/expectation.dart index 1731d2b8f45..0db393b27c6 100644 --- a/pkg/status_file/lib/expectation.dart +++ b/pkg/status_file/lib/expectation.dart @@ -3,7 +3,11 @@ // BSD-style license that can be found in the LICENSE file. /// The possible outcomes from running a test. -class Expectation { +class Expectation._( + final String _name, { + final Expectation? _group, + bool isMeta = false, +}) { /// The test completed normally and did what it intended to do. static final Expectation pass = Expectation._('Pass'); @@ -234,14 +238,8 @@ class Expectation { return expectation; } - final String _name; - final Expectation? _group; - /// Whether this expectation is a test outcome. If not, it's a "meta marker". - final bool isOutcome; - - Expectation._(this._name, {this._group, bool isMeta = false}) - : isOutcome = !isMeta; + final bool isOutcome = !isMeta; bool canBeOutcomeOf(Expectation expectation) { Expectation? outcome = this; diff --git a/pkg/status_file/lib/src/disjunctive.dart b/pkg/status_file/lib/src/disjunctive.dart index 2c5cdd625f6..a751dd4ada3 100644 --- a/pkg/status_file/lib/src/disjunctive.dart +++ b/pkg/status_file/lib/src/disjunctive.dart @@ -143,12 +143,10 @@ List? _satisfiableMinTerms(Expression expression) { /// [variables] are assigned a value bases on the [configuration]. This /// environment can then be used to evaluate the corresponding expression from /// which the [variables] was found. -class TruthTableEnvironment extends Environment { - final List variables; +class TruthTableEnvironment(final List variables) + extends Environment { int configuration = -1; - TruthTableEnvironment(this.variables); - void setConfiguration(int configuration) { this.configuration = configuration; } diff --git a/pkg/status_file/lib/src/expression.dart b/pkg/status_file/lib/src/expression.dart index e6b42cf8136..9b1d47014a4 100644 --- a/pkg/status_file/lib/src/expression.dart +++ b/pkg/status_file/lib/src/expression.dart @@ -8,7 +8,7 @@ import 'dart:math' as math; import '../environment.dart'; /// A parsed Boolean expression AST. -abstract class Expression implements Comparable { +abstract class const Expression() implements Comparable { /// An expression that always evaluates to true. /// /// Used for the section at the top of a status file with no expression. @@ -31,8 +31,6 @@ abstract class Expression implements Comparable { static Expression parse(String expression) => _ExpressionParser(expression).parse(); - const Expression(); - /// Validates that this expression does not contain any invalid uses of /// variables. /// @@ -97,11 +95,7 @@ class _Token { } /// A reference to a variable. -class Variable { - final String name; - - Variable(this.name); - +class Variable(final String name) { String lookup(Environment environment) { var value = environment.lookUp(name); if (value == null) { @@ -125,9 +119,7 @@ class Variable { /// /// Used for the implicit section at the top of a status file that always /// matches. -class _AlwaysExpression extends Expression { - const _AlwaysExpression(); - +class const _AlwaysExpression() extends Expression { @override int _compareToMyType(covariant _AlwaysExpression other) => 0; @@ -149,13 +141,11 @@ class _AlwaysExpression extends Expression { /// $variable == someValue /// ``` /// Negate the result if [negate] is true. -class ComparisonExpression extends Expression { - final Variable left; - final String right; - final bool negate; - - ComparisonExpression(this.left, this.right, this.negate); - +class ComparisonExpression( + final Variable left, + final String right, + final bool negate, +) extends Expression { @override void validate(Environment environment, List errors) { environment.validate(left.name, right, errors); @@ -217,12 +207,8 @@ class ComparisonExpression extends Expression { /// ``` /// $variable != true /// ``` -class VariableExpression extends Expression { - final Variable variable; - final bool negate; - - VariableExpression(this.variable, {this.negate = false}); - +class VariableExpression(final Variable variable, {final bool negate = false}) + extends Expression { @override void validate(Environment environment, List errors) { // It must be a Boolean, so it should allow either Boolean value. @@ -254,16 +240,13 @@ class VariableExpression extends Expression { } /// A logical `||` or `&&` expression. -class LogicExpression extends Expression { +class LogicExpression( /// The operator, `||` or `&&`. - final String op; - - final List operands; - - LogicExpression(this.op, this.operands); - - LogicExpression.and(this.operands) : op = _Token.and; - LogicExpression.or(this.operands) : op = _Token.or; + final String op, + final List operands, +) extends Expression { + new and(List operands) : this(_Token.and, operands); + new or(List operands) : this(_Token.or, operands); bool get isAnd => op == _Token.and; bool get isOr => op == _Token.or; @@ -350,10 +333,8 @@ class LogicExpression extends Expression { } /// Parser for Boolean expressions in a .status file for Dart. -class _ExpressionParser { - final _Scanner _scanner; - - _ExpressionParser(String expression) : _scanner = _Scanner(expression); +class _ExpressionParser(String expression) { + final _Scanner _scanner = _Scanner(expression); Expression parse() { var expression = _parseOr(); @@ -442,7 +423,7 @@ class _ExpressionParser { } /// An iterator that allows peeking at the current token. -class _Scanner { +class _Scanner(String expression) { /// Tokens are "(", ")", "$", "&&", "||", "!", ==", "!=", and (maximal) \w+. static final _testPattern = RegExp(r"^(?:[()$\w\s]|&&|\|\||==|!=?)+$"); static final _tokenPattern = RegExp(r"[()$]|&&|\|\||==|!=?|\w+"); @@ -454,11 +435,11 @@ class _Scanner { static final _identifierPattern = RegExp(r"^\w"); /// The token strings being iterated. - final Iterator tokenIterator; + final Iterator tokenIterator = tokenize(expression).iterator; String? current; - _Scanner(String expression) : tokenIterator = tokenize(expression).iterator { + this { advance(); } diff --git a/pkg/status_file/lib/status_file.dart b/pkg/status_file/lib/status_file.dart index 9d891ccd091..e79f5e96a83 100644 --- a/pkg/status_file/lib/status_file.dart +++ b/pkg/status_file/lib/status_file.dart @@ -49,12 +49,12 @@ class StatusFile { int _lineCount = 0; - StatusFile(this.path); + new(this.path); /// Parses the status file at [path]. /// /// Throws a [SyntaxError] if the file could not be parsed. - StatusFile.read(this.path) { + new read(this.path) { var lines = File(path).readAsLinesSync(); _comments.length = lines.length + 1; @@ -266,7 +266,7 @@ class StatusSection { /// Returns true if this section should apply in the given [environment]. bool isEnabled(Environment environment) => condition.evaluate(environment); - StatusSection(this.condition, this.lineNumber); + new(this.condition, this.lineNumber); } /// Describes the test status of the file or files at a given path. @@ -279,7 +279,7 @@ class StatusEntry { final List expectations; final int? issue; - StatusEntry(this.path, this.lineNumber, this.expectations, this.issue); + new(this.path, this.lineNumber, this.expectations, this.issue); } /// Error thrown when a parse or validation error occurs in a [StatusFile]. @@ -290,7 +290,7 @@ class SyntaxError implements Exception { final String message; final List? errors; - SyntaxError(this.file, this.lineNumber, this.line, this.message, this.errors); + new(this.file, this.lineNumber, this.line, this.message, this.errors); @override String toString() { diff --git a/pkg/status_file/lib/status_file_linter.dart b/pkg/status_file/lib/status_file_linter.dart index 18816d241a4..a430f102005 100644 --- a/pkg/status_file/lib/status_file_linter.dart +++ b/pkg/status_file/lib/status_file_linter.dart @@ -7,15 +7,9 @@ import 'dart:math' as math; import 'package:status_file/canonical_status_file.dart'; import 'package:status_file/status_file_entries_file_checker.dart'; -class LintingError { - final int lineNumber; - final String message; - LintingError(this.lineNumber, this.message); - +class LintingError(final int lineNumber, final String message) { @override - String toString() { - return "Error at line $lineNumber: $message"; - } + String toString() => "Error at line $lineNumber: $message"; } /// Main function to check a status file for linting errors. @@ -268,8 +262,4 @@ ListNotEqualWitness? _findNotEqualWitness(List first, List second) { return null; } -class ListNotEqualWitness { - final T? first; - final T? second; - ListNotEqualWitness(this.first, this.second); -} +class ListNotEqualWitness(final T? first, final T? second); diff --git a/pkg/status_file/pubspec.yaml b/pkg/status_file/pubspec.yaml index a271d70fdbe..30981ba25a6 100644 --- a/pkg/status_file/pubspec.yaml +++ b/pkg/status_file/pubspec.yaml @@ -3,7 +3,7 @@ name: status_file publish_to: none environment: - sdk: '^3.12.0-0' + sdk: '^3.13.0-0' resolution: workspace diff --git a/pkg/status_file/test/status_expression_test.dart b/pkg/status_file/test/status_expression_test.dart index 2d4f8973e61..c34d6e6a85d 100644 --- a/pkg/status_file/test/status_expression_test.dart +++ b/pkg/status_file/test/status_expression_test.dart @@ -10,7 +10,7 @@ import "package:status_file/src/expression.dart"; class TestEnvironment implements Environment { final Map _values; - TestEnvironment(this._values); + new(this._values); @override void validate(String name, String value, List errors) {