diff --git a/dev/bots/analyze.dart b/dev/bots/analyze.dart index 970225ca355..5f52765f6f7 100644 --- a/dev/bots/analyze.dart +++ b/dev/bots/analyze.dart @@ -2365,17 +2365,19 @@ Future verifyNullInitializedDebugExpensiveFields( final _DebugOnlyFieldVisitor visitor = _DebugOnlyFieldVisitor(parsedFile); visitor.visitCompilationUnit(parsedFile.unit); for (final AstNode badNode in visitor.errors) { - errors.add('${file.path}:${parsedFile.lineInfo.getLocation(badNode.offset).lineNumber}'); + errors.add( + '${file.path}:${parsedFile.lineInfo.getLocation(badNode.offset).lineNumber}: fields annotated with @_debugOnly must null initialize.', + ); } } if (errors.isNotEmpty) { foundError([ - '${bold}ERROR: ${red}fields annotated with @_debugOnly must null initialize.$reset', - 'to ensure both the field and initializer are removed from profile/release mode.', - 'These fields should be written as:\n', - 'field = kDebugMode ? : null;\n', - 'Errors were found in the following files:', ...errors, + '', + '$bold${red}Fields annotated with @_debugOnly must null initialize,$reset', + 'to ensure both the field and initializer are removed from profile/release mode.', + 'These fields should be written as:', + 'field = kDebugMode ? : null;', ]); } } @@ -2402,10 +2404,11 @@ Future verifyTabooDocumentation(String workingDirectory, {int minimumMatch } if (errors.isNotEmpty) { foundError([ + ...errors, + '', '${bold}Avoid the word "simply" in documentation. See https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#use-the-passive-voice-recommend-do-not-require-never-say-things-are-simple for details.$reset', '${bold}In many cases these words can be omitted without loss of generality; in other cases it may require a bit of rewording to avoid implying that the task is simple.$reset', '${bold}Similarly, avoid using "note:" or the phrase "note that". See https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#avoid-empty-prose for details.$reset', - ...errors, ]); } } diff --git a/dev/bots/test/analyze-test-input/root/packages/flutter/lib/bar.dart b/dev/bots/test/analyze-test-input/root/packages/flutter/lib/bar.dart index 578d559d310..97699608466 100644 --- a/dev/bots/test/analyze-test-input/root/packages/flutter/lib/bar.dart +++ b/dev/bots/test/analyze-test-input/root/packages/flutter/lib/bar.dart @@ -14,7 +14,7 @@ class Foo { final Map? foo = kDebugMode ? {} : null; @_debugOnly - final Map? bar = kDebugMode ? null : {}; + final Map? bar = kDebugMode ? null : {}; // ERROR: fields annotated with @_debugOnly must null initialize. // dart format off // Checks the annotation works for multiline expressions. @@ -24,36 +24,3 @@ class Foo { : null; // dart format on } - -/// Simply avoid this -/// and simply do that. - -class ClassWithAClampMethod { - ClassWithAClampMethod clamp(double min, double max) => this; -} - -void testNoDoubleClamp(int input) { - final ClassWithAClampMethod nonDoubleClamp = ClassWithAClampMethod(); - // ignore: unnecessary_nullable_for_final_variable_declarations - final ClassWithAClampMethod? nonDoubleClamp2 = nonDoubleClamp; - // ignore: unnecessary_nullable_for_final_variable_declarations - final int? nullableInt = input; - final double? nullableDouble = nullableInt?.toDouble(); - - nonDoubleClamp.clamp(0, 2); - input.clamp(0, 2); - input.clamp(0.0, 2); // bad. - input.toDouble().clamp(0, 2); // bad. - - nonDoubleClamp2?.clamp(0, 2); - nullableInt?.clamp(0, 2); - nullableInt?.clamp(0, 2.0); // bad - nullableDouble?.clamp(0, 2); // bad. - - // ignore: unused_local_variable - final ClassWithAClampMethod Function(double, double)? tearOff1 = nonDoubleClamp2?.clamp; - // ignore: unused_local_variable - final num Function(num, num)? tearOff2 = nullableInt?.clamp; // bad. - // ignore: unused_local_variable - final num Function(num, num)? tearOff3 = nullableDouble?.clamp; // bad. -} diff --git a/dev/bots/test/analyze-test-input/root/packages/flutter/lib/double_clamp.dart b/dev/bots/test/analyze-test-input/root/packages/flutter/lib/double_clamp.dart new file mode 100644 index 00000000000..10611299bea --- /dev/null +++ b/dev/bots/test/analyze-test-input/root/packages/flutter/lib/double_clamp.dart @@ -0,0 +1,33 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +class ClassWithAClampMethod { + ClassWithAClampMethod clamp(double min, double max) => this; +} + +void testNoDoubleClamp(int input) { + final ClassWithAClampMethod nonDoubleClamp = ClassWithAClampMethod(); + // ignore: unnecessary_nullable_for_final_variable_declarations + final ClassWithAClampMethod? nonDoubleClamp2 = nonDoubleClamp; + // ignore: unnecessary_nullable_for_final_variable_declarations + final int? nullableInt = input; + final double? nullableDouble = nullableInt?.toDouble(); + + nonDoubleClamp.clamp(0, 2); + input.clamp(0, 2); + input.clamp(0.0, 2); // ERROR: input.clamp(0.0, 2) + input.toDouble().clamp(0, 2); // ERROR: input.toDouble().clamp(0, 2) + + nonDoubleClamp2?.clamp(0, 2); + nullableInt?.clamp(0, 2); + nullableInt?.clamp(0, 2.0); // ERROR: nullableInt?.clamp(0, 2.0) + nullableDouble?.clamp(0, 2); // ERROR: nullableDouble?.clamp(0, 2) + + // ignore: unused_local_variable + final ClassWithAClampMethod Function(double, double)? tearOff1 = nonDoubleClamp2?.clamp; + // ignore: unused_local_variable + final num Function(num, num)? tearOff2 = nullableInt?.clamp; // ERROR: nullableInt?.clamp + // ignore: unused_local_variable + final num Function(num, num)? tearOff3 = nullableDouble?.clamp; // ERROR: nullableDouble?.clamp +} diff --git a/dev/bots/test/analyze-test-input/root/packages/flutter/lib/renderbox_intrinsics.dart b/dev/bots/test/analyze-test-input/root/packages/flutter/lib/renderbox_intrinsics.dart index 412863a85ee..4342be20da6 100644 --- a/dev/bots/test/analyze-test-input/root/packages/flutter/lib/renderbox_intrinsics.dart +++ b/dev/bots/test/analyze-test-input/root/packages/flutter/lib/renderbox_intrinsics.dart @@ -9,31 +9,32 @@ mixin ARenderBoxMixin on RenderBox { void computeMaxIntrinsicWidth() {} @override - void computeMinIntrinsicWidth() => computeMaxIntrinsicWidth(); // BAD + void computeMinIntrinsicWidth() => computeMaxIntrinsicWidth(); // ERROR: computeMaxIntrinsicWidth(). Consider calling getMaxIntrinsicWidth instead. @override void computeMinIntrinsicHeight() { - final void Function() f = computeMaxIntrinsicWidth; // BAD + final void Function() f = + computeMaxIntrinsicWidth; // ERROR: f = computeMaxIntrinsicWidth. Consider calling getMaxIntrinsicWidth instead. f(); } } extension ARenderBoxExtension on RenderBox { void test() { - computeDryBaseline(); // BAD - computeDryLayout(); // BAD + computeDryBaseline(); // ERROR: computeDryBaseline(). Consider calling getDryBaseline instead. + computeDryLayout(); // ERROR: computeDryLayout(). Consider calling getDryLayout instead. } } class RenderBoxSubclass1 extends RenderBox { @override void computeDryLayout() { - computeDistanceToActualBaseline(); // BAD + computeDistanceToActualBaseline(); // ERROR: computeDistanceToActualBaseline(). Consider calling getDistanceToBaseline, or getDistanceToActualBaseline instead. } @override void computeDistanceToActualBaseline() { - computeMaxIntrinsicHeight(); // BAD + computeMaxIntrinsicHeight(); // ERROR: computeMaxIntrinsicHeight(). Consider calling getMaxIntrinsicHeight instead. } } diff --git a/dev/bots/test/analyze-test-input/root/packages/flutter/lib/stopwatch.dart b/dev/bots/test/analyze-test-input/root/packages/flutter/lib/stopwatch.dart index 722ace367a0..1a919173271 100644 --- a/dev/bots/test/analyze-test-input/root/packages/flutter/lib/stopwatch.dart +++ b/dev/bots/test/analyze-test-input/root/packages/flutter/lib/stopwatch.dart @@ -17,48 +17,48 @@ void testNoStopwatches(Stopwatch stopwatch) { // OK for now, but we probably want to catch public APIs that take a Stopwatch? stopwatch.runtimeType; // Bad: introducing Stopwatch from dart:core. - final Stopwatch localVariable = Stopwatch(); + final Stopwatch localVariable = Stopwatch(); // ERROR: Stopwatch() // Bad: introducing Stopwatch from dart:core. - Stopwatch().runtimeType; + Stopwatch().runtimeType; // ERROR: Stopwatch() (localVariable..runtimeType) // OK: not directly introducing Stopwatch. .runtimeType; // Bad: introducing a Stopwatch subclass. - StopwatchAtHome().runtimeType; + StopwatchAtHome().runtimeType; // ERROR: StopwatchAtHome() // OK: not directly introducing Stopwatch. Stopwatch anotherStopwatch = stopwatch; // Bad: introducing a Stopwatch constructor. - StopwatchAtHome Function() constructor = StopwatchAtHome.new; + StopwatchAtHome Function() constructor = StopwatchAtHome.new; // ERROR: StopwatchAtHome.new assert(() { anotherStopwatch = constructor()..runtimeType; // Bad: introducing a Stopwatch constructor. - constructor = StopwatchAtHome.create; + constructor = StopwatchAtHome.create; // ERROR: StopwatchAtHome.create anotherStopwatch = constructor()..runtimeType; return true; }()); anotherStopwatch.runtimeType; // Bad: introducing an external Stopwatch constructor. - externallib.MyStopwatch.create(); + externallib.MyStopwatch.create(); // ERROR: externallib.MyStopwatch.create() ExternalStopwatchConstructor? externalConstructor; assert(() { // Bad: introducing an external Stopwatch constructor. - externalConstructor = externallib.MyStopwatch.new; + externalConstructor = externallib.MyStopwatch.new; // ERROR: externallib.MyStopwatch.new return true; }()); externalConstructor?.call(); // Bad: introducing an external Stopwatch. - externallib.stopwatch.runtimeType; + externallib.stopwatch.runtimeType; // ERROR: externallib.stopwatch // Bad: calling an external function that returns a Stopwatch. - externallib.createMyStopwatch().runtimeType; + externallib.createMyStopwatch().runtimeType; // ERROR: externallib.createMyStopwatch() // Bad: calling an external function that returns a Stopwatch. - externallib.createStopwatch().runtimeType; + externallib.createStopwatch().runtimeType; // ERROR: externallib.createStopwatch() // Bad: introducing the tear-off form of an external function that returns a Stopwatch. - externalConstructor = externallib.createMyStopwatch; + externalConstructor = externallib.createMyStopwatch; // ERROR: externallib.createMyStopwatch // OK: existing instance. constructor.call().stopwatch; diff --git a/dev/bots/test/analyze-test-input/root/packages/flutter/lib/taboo_words.dart b/dev/bots/test/analyze-test-input/root/packages/flutter/lib/taboo_words.dart new file mode 100644 index 00000000000..99a3dfff6e0 --- /dev/null +++ b/dev/bots/test/analyze-test-input/root/packages/flutter/lib/taboo_words.dart @@ -0,0 +1,7 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +/// Simply avoid this // ERROR: Found use of the taboo word "Simply" in documentation string. +/// and simply do that. // ERROR: Found use of the taboo word "Simply" in documentation string. +T id(T x) => x; diff --git a/dev/bots/test/analyze-test-input/root/packages/foo/deprecation.dart b/dev/bots/test/analyze-test-input/root/packages/foo/deprecation.dart index 022fde5559c..02cb9f9cb65 100644 --- a/dev/bots/test/analyze-test-input/root/packages/foo/deprecation.dart +++ b/dev/bots/test/analyze-test-input/root/packages/foo/deprecation.dart @@ -11,26 +11,26 @@ void test1() {} // The code below is intentionally miss-formatted for testing. // dart format off @Deprecated( - 'bad grammar. ' + 'bad grammar. ' // ERROR: Deprecation notice should be a grammatically correct sentence and start with a capital letter; see style guide: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md 'This feature was deprecated after v1.2.3.' ) void test2() { } @Deprecated( - 'Also bad grammar ' + 'Also bad grammar ' // ERROR: Deprecation notice should be a grammatically correct sentence and end with a period; notice appears to be "Also bad grammar". 'This feature was deprecated after v1.2.3.' ) void test3() { } -@Deprecated('Not the right syntax. This feature was deprecated after v1.2.3.') +@Deprecated('Not the right syntax. This feature was deprecated after v1.2.3.') // ERROR: Deprecation notice must be an adjacent string. void test4() { } -@Deprecated( +@Deprecated( // ERROR: Deprecation notice must be an adjacent string. 'Missing the version line. ' ) void test5() { } -@Deprecated( +@Deprecated( // ERROR: Deprecation notice must be an adjacent string. 'This feature was deprecated after v1.2.3.' ) void test6() { } @@ -49,13 +49,13 @@ void test8() { } @Deprecated( 'Version number test (should fail). ' - 'This feature was deprecated after v1.20.0.' + 'This feature was deprecated after v1.20.0.' // ERROR: Deprecation notice does not accurately indicate a beta branch version number; please see https://flutter.dev/docs/development/tools/sdk/releases to find the latest beta build version number. ) void test9() { } @Deprecated( 'Version number test (should fail). ' - 'This feature was deprecated after v1.21.0.' + 'This feature was deprecated after v1.21.0.' // ERROR: Deprecation notice does not accurately indicate a beta branch version number; please see https://flutter.dev/docs/development/tools/sdk/releases to find the latest beta build version number. ) void test10() { } @@ -78,7 +78,7 @@ void test12() { } void test13() { } @Deprecated( - "Double quotes' test (should fail). " + "Double quotes' test (should fail). " // ERROR: Deprecation notice does not match required pattern. You might have used double quotes (") for the string instead of single quotes ('). 'This feature was deprecated after v2.1.0-11.0.pre.' ) void test14() { } diff --git a/dev/bots/test/analyze-test-input/root/packages/foo/meta.dart b/dev/bots/test/analyze-test-input/root/packages/foo/meta.dart new file mode 100644 index 00000000000..b1e97489166 --- /dev/null +++ b/dev/bots/test/analyze-test-input/root/packages/foo/meta.dart @@ -0,0 +1,6 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// ERROR: error #1 // ERROR: error #2 +// ERROR: error #3 diff --git a/dev/bots/test/analyze_test.dart b/dev/bots/test/analyze_test.dart index d5510f9c83f..5ea0559003c 100644 --- a/dev/bots/test/analyze_test.dart +++ b/dev/bots/test/analyze_test.dart @@ -54,40 +54,34 @@ void main() { ); final String testGenDefaultsPath = path.join('test', 'analyze-gen-defaults'); + test('matchesErrorsInFile matcher basic test', () async { + final String result = await capture(() async { + foundError([ + 'meta.dart:5: error #1', + 'meta.dart:5: error #2', + 'meta.dart:6: error #3', + '', + 'Error summary', + ]); + }, shouldHaveErrors: true); + final File fixture = File(path.join(testRootPath, 'packages', 'foo', 'meta.dart')); + expect(result, matchesErrorsInFile(fixture, endsWith: ['', 'Error summary'])); + }); + test('analyze.dart - verifyDeprecations', () async { final String result = await capture( () => verifyDeprecations(testRootPath, minimumMatches: 2), shouldHaveErrors: true, ); - final String lines = [ - '║ test/analyze-test-input/root/packages/foo/deprecation.dart:14: Deprecation notice should be a grammatically correct sentence and start with a capital letter; see style guide: STYLE_GUIDE_URL', - '║ test/analyze-test-input/root/packages/foo/deprecation.dart:20: Deprecation notice should be a grammatically correct sentence and end with a period; notice appears to be "Also bad grammar".', - '║ test/analyze-test-input/root/packages/foo/deprecation.dart:25: Deprecation notice must be an adjacent string.', - '║ test/analyze-test-input/root/packages/foo/deprecation.dart:28: Deprecation notice must be an adjacent string.', - '║ test/analyze-test-input/root/packages/foo/deprecation.dart:33: Deprecation notice must be an adjacent string.', - '║ test/analyze-test-input/root/packages/foo/deprecation.dart:52: Deprecation notice does not accurately indicate a beta branch version number; please see RELEASES_URL to find the latest beta build version number.', - '║ test/analyze-test-input/root/packages/foo/deprecation.dart:58: Deprecation notice does not accurately indicate a beta branch version number; please see RELEASES_URL to find the latest beta build version number.', - '║ test/analyze-test-input/root/packages/foo/deprecation.dart:81: Deprecation notice does not match required pattern. You might have used double quotes (") for the string instead of single quotes (\').', - ] - .map((String line) { - return line - .replaceAll('/', Platform.isWindows ? r'\' : '/') - .replaceAll( - 'STYLE_GUIDE_URL', - 'https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md', - ) - .replaceAll( - 'RELEASES_URL', - 'https://flutter.dev/docs/development/tools/sdk/releases', - ); - }) - .join('\n'); + final File fixture = File(path.join(testRootPath, 'packages', 'foo', 'deprecation.dart')); expect( result, - '╔═╡ERROR #1╞════════════════════════════════════════════════════════════════════\n' - '$lines\n' - '║ See: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes\n' - '╚═══════════════════════════════════════════════════════════════════════════════\n', + matchesErrorsInFile( + fixture, + endsWith: [ + 'See: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes', + ], + ), ); }); @@ -279,15 +273,20 @@ void main() { shouldHaveErrors: true, ); - expect(result, isNot(contains(':13'))); - expect(result, isNot(contains(':14'))); - expect(result, isNot(contains(':15'))); - expect(result, isNot(contains(':19'))); - expect(result, isNot(contains(':20'))); - expect(result, isNot(contains(':21'))); - expect(result, isNot(contains(':22'))); - - expect(result, contains(':17')); + final File fixture = File(path.join(testRootPath, 'packages', 'flutter', 'lib', 'bar.dart')); + expect( + result, + matchesErrorsInFile( + fixture, + endsWith: [ + '', + 'Fields annotated with @_debugOnly must null initialize,', + 'to ensure both the field and initializer are removed from profile/release mode.', + 'These fields should be written as:', + 'field = kDebugMode ? : null;', + ], + ), + ); }); test('analyze.dart - verifyTabooDocumentation', () async { @@ -296,9 +295,21 @@ void main() { shouldHaveErrors: true, ); - expect(result, isNot(contains(':27'))); - expect(result, contains(':28')); - expect(result, contains(':29')); + final File fixture = File( + path.join(testRootPath, 'packages', 'flutter', 'lib', 'taboo_words.dart'), + ); + expect( + result, + matchesErrorsInFile( + fixture, + endsWith: [ + '', + 'Avoid the word "simply" in documentation. See https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#use-the-passive-voice-recommend-do-not-require-never-say-things-are-simple for details.', + 'In many cases these words can be omitted without loss of generality; in other cases it may require a bit of rewording to avoid implying that the task is simple.', + 'Similarly, avoid using "note:" or the phrase "note that". See https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#avoid-empty-prose for details.', + ], + ), + ); }); test('analyze.dart - clampDouble', () async { @@ -310,21 +321,19 @@ void main() { ), shouldHaveErrors: true, ); - final String lines = [ - '║ packages/flutter/lib/bar.dart:45: input.clamp(0.0, 2)', - '║ packages/flutter/lib/bar.dart:46: input.toDouble().clamp(0, 2)', - '║ packages/flutter/lib/bar.dart:50: nullableInt?.clamp(0, 2.0)', - '║ packages/flutter/lib/bar.dart:51: nullableDouble?.clamp(0, 2)', - '║ packages/flutter/lib/bar.dart:56: nullableInt?.clamp', - '║ packages/flutter/lib/bar.dart:58: nullableDouble?.clamp', - ].map((String line) => line.replaceAll('/', Platform.isWindows ? r'\' : '/')).join('\n'); + + final File fixture = File( + path.join(testRootPath, 'packages', 'flutter', 'lib', 'double_clamp.dart'), + ); expect( result, - '╔═╡ERROR #1╞════════════════════════════════════════════════════════════════════\n' - '$lines\n' - '║ \n' - '║ For performance reasons, we use a custom "clampDouble" function instead of using "double.clamp".\n' - '╚═══════════════════════════════════════════════════════════════════════════════\n', + matchesErrorsInFile( + fixture, + endsWith: [ + '', // empty line before the last sentence. + 'For performance reasons, we use a custom "clampDouble" function instead of using "double.clamp".', + ], + ), ); }); @@ -337,27 +346,20 @@ void main() { ), shouldHaveErrors: true, ); - final String lines = [ - '║ packages/flutter/lib/stopwatch.dart:20: Stopwatch()', - '║ packages/flutter/lib/stopwatch.dart:22: Stopwatch()', - '║ packages/flutter/lib/stopwatch.dart:28: StopwatchAtHome()', - '║ packages/flutter/lib/stopwatch.dart:33: StopwatchAtHome.new', - '║ packages/flutter/lib/stopwatch.dart:37: StopwatchAtHome.create', - '║ packages/flutter/lib/stopwatch.dart:44: externallib.MyStopwatch.create()', - '║ packages/flutter/lib/stopwatch.dart:49: externallib.MyStopwatch.new', - '║ packages/flutter/lib/stopwatch.dart:55: externallib.stopwatch', - '║ packages/flutter/lib/stopwatch.dart:57: externallib.createMyStopwatch()', - '║ packages/flutter/lib/stopwatch.dart:59: externallib.createStopwatch()', - '║ packages/flutter/lib/stopwatch.dart:61: externallib.createMyStopwatch', - ].map((String line) => line.replaceAll('/', Platform.isWindows ? r'\' : '/')).join('\n'); + + final File fixture = File( + path.join(testRootPath, 'packages', 'flutter', 'lib', 'stopwatch.dart'), + ); expect( result, - '╔═╡ERROR #1╞════════════════════════════════════════════════════════════════════\n' - '$lines\n' - '║ \n' - '║ Stopwatches introduce flakes by falling out of sync with the FakeAsync used in testing.\n' - '║ A Stopwatch that stays in sync with FakeAsync is available through the Gesture or Test bindings, through samplingClock.\n' - '╚═══════════════════════════════════════════════════════════════════════════════\n', + matchesErrorsInFile( + fixture, + endsWith: [ + '', + 'Stopwatches introduce flakes by falling out of sync with the FakeAsync used in testing.', + 'A Stopwatch that stays in sync with FakeAsync is available through the Gesture or Test bindings, through samplingClock.', + ], + ), ); }); @@ -370,21 +372,18 @@ void main() { ), shouldHaveErrors: true, ); - final String lines = [ - '║ packages/flutter/lib/renderbox_intrinsics.dart:12: computeMaxIntrinsicWidth(). Consider calling getMaxIntrinsicWidth instead.', - '║ packages/flutter/lib/renderbox_intrinsics.dart:16: f = computeMaxIntrinsicWidth. Consider calling getMaxIntrinsicWidth instead.', - '║ packages/flutter/lib/renderbox_intrinsics.dart:23: computeDryBaseline(). Consider calling getDryBaseline instead.', - '║ packages/flutter/lib/renderbox_intrinsics.dart:24: computeDryLayout(). Consider calling getDryLayout instead.', - '║ packages/flutter/lib/renderbox_intrinsics.dart:31: computeDistanceToActualBaseline(). Consider calling getDistanceToBaseline, or getDistanceToActualBaseline instead.', - '║ packages/flutter/lib/renderbox_intrinsics.dart:36: computeMaxIntrinsicHeight(). Consider calling getMaxIntrinsicHeight instead.', - ].map((String line) => line.replaceAll('/', Platform.isWindows ? r'\' : '/')).join('\n'); + final File fixture = File( + path.join(testRootPath, 'packages', 'flutter', 'lib', 'renderbox_intrinsics.dart'), + ); expect( result, - '╔═╡ERROR #1╞════════════════════════════════════════════════════════════════════\n' - '$lines\n' - '║ \n' - '║ Typically the get* methods should be used to obtain the intrinsics of a RenderBox.\n' - '╚═══════════════════════════════════════════════════════════════════════════════\n', + matchesErrorsInFile( + fixture, + endsWith: [ + '', + 'Typically the get* methods should be used to obtain the intrinsics of a RenderBox.', + ], + ), ); }); diff --git a/dev/bots/test/common.dart b/dev/bots/test/common.dart index 0a697ad8223..67088acfcf2 100644 --- a/dev/bots/test/common.dart +++ b/dev/bots/test/common.dart @@ -3,9 +3,13 @@ // found in the LICENSE file. import 'dart:io'; +import 'dart:math' as math; +import 'package:collection/collection.dart'; import 'package:test/test.dart'; +import '../utils.dart'; + export 'package:test/test.dart' hide isInstanceOf; /// A matcher that compares the type of the actual value to the type argument T. @@ -31,3 +35,188 @@ Matcher throwsExceptionWith(String messageSubString) { ), ); } + +/// A matcher that matches error messages specified in the given `fixture` [File]. +/// +/// This matcher allows analyzer tests to specify the expected error messages +/// in the test fixture file, eliminating the need to hard code line numbers in +/// the test. +/// +/// The error messages must be printed using the [foundError] function. Each +/// error must start with the path to the file where the error resides, line +/// number (1-based instead of 0-based) of the error, and a short description, +/// delimited by colons (`:`). In the test fixture one could add the following +/// comment on the line that would produce the error, to tell matcher what to +/// expect: +/// `// ERROR: `. +Matcher matchesErrorsInFile(File fixture, {List endsWith = const []}) => + _ErrorMatcher(fixture, endsWith); + +class _ErrorMatcher extends Matcher { + _ErrorMatcher(this.file, this.endsWith) : bodyMatcher = _ErrorsInFileMatcher(file); + + static const String mismatchDescriptionKey = 'mismatchDescription'; + static final int _errorBoxWidth = math.max(15, (hasColor ? stdout.terminalColumns : 80) - 1); + static const String _title = 'ERROR #1'; + static final String _firstLine = '╔═╡$_title╞═${"═" * (_errorBoxWidth - 4 - _title.length)}'; + + static final String _lastLine = '╚${"═" * _errorBoxWidth}'; + static const String _linePrefix = '║ '; + + static bool mismatch(String mismatchDescription, Map matchState) { + matchState[mismatchDescriptionKey] = mismatchDescription; + return false; + } + + final List endsWith; + final File file; + final _ErrorsInFileMatcher bodyMatcher; + + @override + bool matches(dynamic item, Map matchState) { + if (item is! String) { + return mismatch('expected a String, got $item', matchState); + } + final List lines = item.split('\n'); + if (lines.isEmpty) { + return mismatch('the actual error message is empty', matchState); + } + if (lines.first != _firstLine) { + return mismatch( + 'the first line of the error message must be $_firstLine, got ${lines.first}', + matchState, + ); + } + if (lines.last.isNotEmpty) { + return mismatch( + 'missing newline at the end of the error message, got ${lines.last}', + matchState, + ); + } + if (lines[lines.length - 2] != _lastLine) { + return mismatch( + 'the last line of the error message must be $_lastLine, got ${lines[lines.length - 2]}', + matchState, + ); + } + final List body = lines.sublist(1, lines.length - 2); + final String? noprefix = body.firstWhereOrNull((String line) => !line.startsWith(_linePrefix)); + if (noprefix != null) { + return mismatch( + 'Line "$noprefix" should start with a prefix $_linePrefix..\n$lines', + matchState, + ); + } + + final List bodyWithoutPrefix = body + .map((String s) => s.substring(_linePrefix.length)) + .toList(growable: false); + final bool hasTailMismatch = IterableZip(>[ + bodyWithoutPrefix.reversed, + endsWith.reversed, + ]).any((List ss) => ss[0] != ss[1]); + if (bodyWithoutPrefix.length < endsWith.length || hasTailMismatch) { + return mismatch( + 'The error message should end with $endsWith.\n' + 'Actual error(s): $item', + matchState, + ); + } + return bodyMatcher.matches( + bodyWithoutPrefix.sublist(0, bodyWithoutPrefix.length - endsWith.length), + matchState, + ); + } + + @override + Description describe(Description description) { + return description.add('file ${file.path} contains the expected analyze errors.'); + } + + @override + Description describeMismatch( + dynamic item, + Description mismatchDescription, + Map matchState, + bool verbose, + ) { + final String? description = matchState[mismatchDescriptionKey] as String?; + return description != null + ? mismatchDescription.add(description) + : mismatchDescription.add('$matchState'); + } +} + +class _ErrorsInFileMatcher extends Matcher { + _ErrorsInFileMatcher(this.file); + + final File file; + + static final RegExp expectationMatcher = RegExp(r'// ERROR: (?.+)$'); + + static bool mismatch(String mismatchDescription, Map matchState) { + return _ErrorMatcher.mismatch(mismatchDescription, matchState); + } + + List<(int, String)> _expectedErrorMessagesFromFile(Map matchState) { + final List<(int, String)> returnValue = <(int, String)>[]; + for (final (int index, String line) in file.readAsLinesSync().indexed) { + final List expectations = + expectationMatcher.firstMatch(line)?.namedGroup('expectations')?.split(' // ERROR: ') ?? + []; + for (final String expectation in expectations) { + returnValue.add((index + 1, expectation)); + } + } + return returnValue; + } + + @override + bool matches(dynamic item, Map matchState) { + final List actualErrors = item as List; + final List<(int, String)> expectedErrors = _expectedErrorMessagesFromFile(matchState); + if (expectedErrors.length != actualErrors.length) { + return mismatch( + 'expected ${expectedErrors.length} error(s), got ${actualErrors.length}.\n' + 'expected lines with errors: ${expectedErrors.map(((int, String) x) => x.$1).toList()}\n' + 'actual error(s): \n>${actualErrors.join('\n>')}', + matchState, + ); + } + for (int i = 0; i < actualErrors.length; ++i) { + final String actualError = actualErrors[i]; + final (int lineNumber, String expectedError) = expectedErrors[i]; + switch (actualError.split(':')) { + case [final String _]: + return mismatch('No colons (":") found in the error message "$actualError".', matchState); + case [final String path, final String line, ...final List rest]: + if (!path.endsWith(file.uri.pathSegments.last)) { + return mismatch('"$path" does not match the file name of the source file.', matchState); + } + if (lineNumber.toString() != line) { + return mismatch( + 'could not find the expected error "$expectedError" at line $lineNumber', + matchState, + ); + } + final String actualMessage = rest.join(':').trimLeft(); + if (actualMessage != expectedError) { + return mismatch( + 'expected \n"$expectedError"\n at line $lineNumber, got \n"$actualMessage"', + matchState, + ); + } + + case _: + return mismatch( + 'failed to recognize a valid path from the error message "$actualError".', + matchState, + ); + } + } + return true; + } + + @override + Description describe(Description description) => description; +}