diff --git a/dev/bots/analyze.dart b/dev/bots/analyze.dart index e79fbd056c63abaca32d92ccaea27b89168bce6e..970225ca3550e93dd5e22b4f65faa957f51d9d30 100644 --- a/dev/bots/analyze.dart +++ b/dev/bots/analyze.dart @@ -694,18 +694,17 @@ class _DeprecationMessagesVisitor extends RecursiveAstVisitor<void> { '// flutter_ignore: deprecation_syntax (see analyze.dart)'; /// Some deprecation notices are exempt for historical reasons. They must have an issue listed. - final RegExp legacyDeprecation = RegExp( - r'// flutter_ignore: deprecation_syntax, https://github.com/flutter/flutter/issues/\d+$', + static final RegExp legacyDeprecation = RegExp( + r'// flutter_ignore: deprecation_syntax, https://github.com/flutter/flutter/issues/\d+', ); - final RegExp _deprecationMessagePattern = RegExp(r"^ *'(?<message>.+) '$"); - final RegExp _deprecationVersionPattern = RegExp( - r"'This feature was deprecated after v(?<major>\d+)\.(?<minor>\d+)\.(?<patch>\d+)(?<build>-\d+\.\d+\.pre)?\.',?$", + static final RegExp deprecationVersionPattern = RegExp( + r'This feature was deprecated after v(?<major>\d+)\.(?<minor>\d+)\.(?<patch>\d+)(?<build>-\d+\.\d+\.pre)?\.$', ); - String _errorWithLineInfo(AstNode node, {required String error}) { + void _addErrorWithLineInfo(AstNode node, {required String error}) { final int lineNumber = parseResult.lineInfo.getLocation(node.offset).lineNumber; - return '$filePath:$lineNumber: $error'; + errors.add('$filePath:$lineNumber: $error'); } @override @@ -718,111 +717,107 @@ class _DeprecationMessagesVisitor extends RecursiveAstVisitor<void> { if (!shouldCheckAnnotation) { return; } - final NodeList<StringLiteral> strings; - try { - strings = switch (node.arguments?.arguments) { - null || NodeList<Expression>(first: AdjacentStrings(strings: []), length: 1) => - throw _errorWithLineInfo( - node, - error: - '@Deprecated annotation should take exactly one string as parameter, got ${node.arguments}', - ), - NodeList<Expression>( - first: AdjacentStrings(:final NodeList<StringLiteral> strings), - length: 1, - ) => - strings, - final NodeList<Expression> expressions => - throw _errorWithLineInfo( - node, - error: - '@Deprecated annotation should take exactly one string as parameter, but got $expressions', - ), - }; - } catch (error) { - errors.add(error.toString()); + final NodeList<Expression>? arguments = node.arguments?.arguments; + if (arguments == null || arguments.length != 1) { + _addErrorWithLineInfo( + node, + error: 'A @Deprecation annotation must have exactly one deprecation notice String.', + ); return; } - + final Expression deprecationNotice = arguments.first; + if (deprecationNotice is! AdjacentStrings) { + _addErrorWithLineInfo(node, error: 'Deprecation notice must be an adjacent string.'); + return; + } + final List<StringLiteral> strings = deprecationNotice.strings; final Iterator<StringLiteral> deprecationMessageIterator = strings.iterator; final bool isNotEmpty = deprecationMessageIterator.moveNext(); - assert(isNotEmpty); - - try { - RegExpMatch? versionMatch; - String? message; - do { - final StringLiteral deprecationString = deprecationMessageIterator.current; - final String line = deprecationString.toSource(); - final RegExpMatch? messageMatch = _deprecationMessagePattern.firstMatch(line); - if (messageMatch == null) { - String possibleReason = ''; - if (line.trimLeft().startsWith('"')) { - possibleReason = - ' You might have used double quotes (") for the string instead of single quotes (\').'; - } else if (!line.contains("'")) { - possibleReason = - ' It might be missing the line saying "This feature was deprecated after...".'; - } else if (!line.trimRight().endsWith(" '")) { - if (line.contains('This feature was deprecated')) { - possibleReason = ' There might not be an explanatory message.'; - } else { - possibleReason = ' There might be a missing space character at the end of the line.'; - } - } - throw _errorWithLineInfo( - deprecationString, - error: 'Deprecation notice does not match required pattern.$possibleReason', - ); - } - if (message == null) { - message = messageMatch.namedGroup('message'); - final String firstChar = String.fromCharCode(message!.runes.first); - if (firstChar.toUpperCase() != firstChar) { - throw _errorWithLineInfo( - deprecationString, - 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', - ); - } - } else { - message += messageMatch.namedGroup('message')!; - } - if (!deprecationMessageIterator.moveNext()) { - throw _errorWithLineInfo( - deprecationString, - error: ' It might be missing the line saying "This feature was deprecated after...".', - ); - } - versionMatch = _deprecationVersionPattern.firstMatch( - deprecationMessageIterator.current.toSource(), + assert(isNotEmpty); // An AdjacentString always has 2 or more string literals. + + final [...List<StringLiteral> messageLiterals, StringLiteral versionLiteral] = strings; + + // Verify the version literal has the correct pattern. + final RegExpMatch? versionMatch = + versionLiteral is SimpleStringLiteral + ? deprecationVersionPattern.firstMatch(versionLiteral.value) + : null; + if (versionMatch == null) { + _addErrorWithLineInfo( + versionLiteral, + error: + 'Deprecation notice must end with a line saying "This feature was deprecated after...".', + ); + return; + } + + final int major = int.parse(versionMatch.namedGroup('major')!); + final int minor = int.parse(versionMatch.namedGroup('minor')!); + final int patch = int.parse(versionMatch.namedGroup('patch')!); + final bool hasBuild = versionMatch.namedGroup('build') != null; + // There was a beta release that was mistakenly labeled 3.1.0 without a build. + final bool specialBeta = major == 3 && minor == 1 && patch == 0; + if (!specialBeta && (major > 1 || (major == 1 && minor >= 20))) { + if (!hasBuild) { + _addErrorWithLineInfo( + versionLiteral, + 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.', ); - } while (versionMatch == null); - - final int major = int.parse(versionMatch.namedGroup('major')!); - final int minor = int.parse(versionMatch.namedGroup('minor')!); - final int patch = int.parse(versionMatch.namedGroup('patch')!); - final bool hasBuild = versionMatch.namedGroup('build') != null; - // There was a beta release that was mistakenly labeled 3.1.0 without a build. - final bool specialBeta = major == 3 && minor == 1 && patch == 0; - if (!specialBeta && (major > 1 || (major == 1 && minor >= 20))) { - if (!hasBuild) { - throw _errorWithLineInfo( - deprecationMessageIterator.current, - 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.', - ); - } + return; } - if (!message.endsWith('.') && !message.endsWith('!') && !message.endsWith('?')) { - throw _errorWithLineInfo( - node, + } + + // Verify the version literal has the correct pattern. + assert(messageLiterals.isNotEmpty); // An AdjacentString always has 2 or more string literals. + for (final StringLiteral message in messageLiterals) { + if (message is! SingleStringLiteral) { + _addErrorWithLineInfo( + message, + error: 'Deprecation notice does not match required pattern.', + ); + return; + } + if (!message.isSingleQuoted) { + _addErrorWithLineInfo( + message, error: - 'Deprecation notice should be a grammatically correct sentence and end with a period; notice appears to be "$message".', + 'Deprecation notice does not match required pattern. You might have used double quotes (") for the string instead of single quotes (\').', ); + return; } - } catch (error) { - errors.add(error.toString()); + } + final String fullExplanation = + messageLiterals + .map((StringLiteral message) => message.stringValue ?? '') + .join() + .trimRight(); + if (fullExplanation.isEmpty) { + _addErrorWithLineInfo( + messageLiterals.last, + error: + 'Deprecation notice should be a grammatically correct sentence and end with a period; There might not be an explanatory message.', + ); + return; + } + final String firstChar = String.fromCharCode(fullExplanation.runes.first); + if (firstChar.toUpperCase() != firstChar) { + _addErrorWithLineInfo( + messageLiterals.first, + 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', + ); + return; + } + if (!fullExplanation.endsWith('.') && + !fullExplanation.endsWith('?') && + !fullExplanation.endsWith('!')) { + _addErrorWithLineInfo( + messageLiterals.last, + error: + 'Deprecation notice should be a grammatically correct sentence and end with a period; notice appears to be "$fullExplanation".', + ); + return; } } } @@ -2339,10 +2334,7 @@ class _DebugOnlyFieldVisitor extends RecursiveAstVisitor<void> { final List<AstNode> errors = <AstNode>[]; static const String _kDebugOnlyAnnotation = '_debugOnly'; - static final RegExp _nullInitializedField = RegExp( - r'kDebugMode \? [\w<> ,{}()]+ : null;', - multiLine: true, - ); + static final RegExp _nullInitializedField = RegExp(r'kDebugMode \? [\w<> ,{}()]+ : null;'); @override void visitFieldDeclaration(FieldDeclaration node) { @@ -2351,7 +2343,7 @@ class _DebugOnlyFieldVisitor extends RecursiveAstVisitor<void> { (Annotation annotation) => annotation.name.name == _kDebugOnlyAnnotation, )) { if (!node.toSource().contains(_nullInitializedField)) { - errors.add(node); + errors.add(node.fields); // Use the fields node for line number. } } } 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 fa9c79eb9b7c85789d8f85fae0c02af2a085404d..578d559d3106a0be9ef06269dab6b280fce50cbd 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 @@ -15,6 +15,14 @@ class Foo { @_debugOnly final Map<String, String>? bar = kDebugMode ? null : <String, String>{}; + + // dart format off + // Checks the annotation works for multiline expressions. + @_debugOnly + final Map<String, String>? multiline = kDebugMode + ? <String, String>{} + : null; + // dart format on } /// Simply avoid this 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 1067a501399817744766dc3792f21b2c8a36130a..022fde5559c645f8d9d6b95832cf233ce533a00f 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 @@ -10,108 +10,88 @@ void test1() {} // The code below is intentionally miss-formatted for testing. // dart format off -@Deprecated( - 'Missing space ->.' //ignore: missing_whitespace_between_adjacent_strings - 'This feature was deprecated after v1.2.3.' -) -void test2() { } - @Deprecated( 'bad grammar. ' 'This feature was deprecated after v1.2.3.' ) -void test3() { } +void test2() { } @Deprecated( 'Also bad grammar ' 'This feature was deprecated after v1.2.3.' ) -void test4() { } - -@deprecated // ignore: provide_deprecation_message -void test5() { } +void test3() { } @Deprecated('Not the right syntax. This feature was deprecated after v1.2.3.') -void test6() { } +void test4() { } @Deprecated( 'Missing the version line. ' ) -void test7() { } - -@Deprecated( - 'This feature was deprecated after v1.2.3.' -) -void test8() { } +void test5() { } @Deprecated( - 'Not the right syntax. ' 'This feature was deprecated after v1.2.3.' -) void test9() { } - -@Deprecated( - 'Not the right syntax. ' - 'This feature was deprecated after v1.2.3.' ) -void test10() { } +void test6() { } @Deprecated( 'URLs are not required. ' 'This feature was deprecated after v1.0.0.' ) -void test11() { } +void test7() { } @Deprecated( - 'Version number test (should fail). ' + 'Version number test (should pass). ' 'This feature was deprecated after v1.19.0.' ) -void test12() { } +void test8() { } @Deprecated( 'Version number test (should fail). ' 'This feature was deprecated after v1.20.0.' ) -void test13() { } +void test9() { } @Deprecated( 'Version number test (should fail). ' 'This feature was deprecated after v1.21.0.' ) -void test14() { } +void test10() { } @Deprecated( 'Version number test (special beta should pass). ' 'This feature was deprecated after v3.1.0.' ) -void test15() { } +void test11() { } @Deprecated( 'Version number test (should be fine). ' 'This feature was deprecated after v0.1.0.' ) -void test16() { } +void test12() { } @Deprecated( 'Version number test (should be fine). ' 'This feature was deprecated after v1.20.0-1.0.pre.' ) -void test17() { } +void test13() { } @Deprecated( "Double quotes' test (should fail). " 'This feature was deprecated after v2.1.0-11.0.pre.' ) -void test18() { } +void test14() { } @Deprecated( // flutter_ignore: deprecation_syntax, https://github.com/flutter/flutter/issues/000000 'Missing the version line. ' ) -void test19() { } +void test15() { } // dart format on // flutter_ignore: deprecation_syntax, https://github.com/flutter/flutter/issues/000000 @Deprecated('Missing the version line. ') -void test20() {} +void test16() {} class TestClass1 { // flutter_ignore: deprecation_syntax, https://github.com/flutter/flutter/issues/000000 diff --git a/dev/bots/test/analyze_test.dart b/dev/bots/test/analyze_test.dart index fbff036dd958e76ea4510be782d7f4519c7e4383..d5510f9c83fd686619e4eb6356e1121b8b2509f8 100644 --- a/dev/bots/test/analyze_test.dart +++ b/dev/bots/test/analyze_test.dart @@ -54,51 +54,42 @@ void main() { ); final String testGenDefaultsPath = path.join('test', 'analyze-gen-defaults'); - test( - 'analyze.dart - verifyDeprecations', - () async { - final String result = await capture( - () => verifyDeprecations(testRootPath, minimumMatches: 2), - shouldHaveErrors: true, - ); - final String lines = <String>[ - 'â•‘ test/analyze-test-input/root/packages/foo/deprecation.dart:14: Deprecation notice does not match required pattern. There might be a missing space character at the end of the line.', - 'â•‘ test/analyze-test-input/root/packages/foo/deprecation.dart:20: 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:27: 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:31: Deprecation notice does not match required pattern.', - 'â•‘ test/analyze-test-input/root/packages/foo/deprecation.dart:34: Deprecation notice does not match required pattern.', - 'â•‘ test/analyze-test-input/root/packages/foo/deprecation.dart:39: Deprecation notice does not match required pattern. It might be missing the line saying "This feature was deprecated after...".', - 'â•‘ test/analyze-test-input/root/packages/foo/deprecation.dart:43: Deprecation notice does not match required pattern. There might not be an explanatory message.', - 'â•‘ test/analyze-test-input/root/packages/foo/deprecation.dart:50: End of deprecation notice does not match required pattern.', - 'â•‘ test/analyze-test-input/root/packages/foo/deprecation.dart:53: Unexpected deprecation notice indent.', - 'â•‘ test/analyze-test-input/root/packages/foo/deprecation.dart:72: 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:78: 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:101: 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'); - 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', - ); - }, - // TODO(goderbauer): Update and re-enable this after formatting changes have landed. - skip: true, - ); + test('analyze.dart - verifyDeprecations', () async { + final String result = await capture( + () => verifyDeprecations(testRootPath, minimumMatches: 2), + shouldHaveErrors: true, + ); + final String lines = <String>[ + 'â•‘ 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'); + 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', + ); + }); test('analyze.dart - verifyGoldenTags', () async { final List<String> result = (await capture( @@ -288,8 +279,15 @@ void main() { shouldHaveErrors: true, ); - expect(result, contains(':16')); 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')); }); test('analyze.dart - verifyTabooDocumentation', () async { @@ -298,9 +296,9 @@ void main() { shouldHaveErrors: true, ); - expect(result, isNot(contains(':19'))); - expect(result, contains(':20')); - expect(result, contains(':21')); + expect(result, isNot(contains(':27'))); + expect(result, contains(':28')); + expect(result, contains(':29')); }); test('analyze.dart - clampDouble', () async { @@ -313,12 +311,12 @@ void main() { shouldHaveErrors: true, ); final String lines = <String>[ - 'â•‘ packages/flutter/lib/bar.dart:37: input.clamp(0.0, 2)', - 'â•‘ packages/flutter/lib/bar.dart:38: input.toDouble().clamp(0, 2)', - 'â•‘ packages/flutter/lib/bar.dart:42: nullableInt?.clamp(0, 2.0)', - 'â•‘ packages/flutter/lib/bar.dart:43: nullableDouble?.clamp(0, 2)', - 'â•‘ packages/flutter/lib/bar.dart:48: nullableInt?.clamp', - 'â•‘ packages/flutter/lib/bar.dart:50: nullableDouble?.clamp', + 'â•‘ 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'); expect( result, diff --git a/dev/bots/utils.dart b/dev/bots/utils.dart index 429d9e13ad77f4d2f12a312b1cdd1d2606a0aa5b..c14e01582906e59128a9ef4915ba6b385438190a 100644 --- a/dev/bots/utils.dart +++ b/dev/bots/utils.dart @@ -377,10 +377,7 @@ bool hasInlineIgnore( return false; } return compilationUnit.content - .substring( - lineInfo.getOffsetOfLine(lineNumber - 1), - lineInfo.getOffsetOfLine(lineNumber) - 1, // Excludes LF, see the comment above. - ) + .substring(lineInfo.getOffsetOfLine(lineNumber - 1), lineInfo.getOffsetOfLine(lineNumber)) .trimLeft() .contains(ignoreDirectivePattern); }