Unverified Commit f1013e68 authored by includecmath's avatar includecmath Committed by GitHub

[flutter_tools] According to AnalysisSeverity return exit code detailed proposal (#61589)

No matter what level(error, warning, info) issues flutter analyze always return fatal exit code(1). CI/CD environment receive 1(!0). This may leads to e.g. Jenkins Build step 'Execute shell' marked build as failure.
I propose according to AnalysisSeverity level return fatal(1) or success(0) exit code.
parent db2532a9
...@@ -73,6 +73,14 @@ class AnalyzeCommand extends FlutterCommand { ...@@ -73,6 +73,14 @@ class AnalyzeCommand extends FlutterCommand {
help: 'When analyzing the flutter repository, display the number of ' help: 'When analyzing the flutter repository, display the number of '
'files that will be analyzed.\n' 'files that will be analyzed.\n'
'Ignored if --watch is specified.'); 'Ignored if --watch is specified.');
argParser.addFlag('fatal-infos',
negatable: true,
help: 'Treat info level issues as fatal.',
defaultsTo: true);
argParser.addFlag('fatal-warnings',
negatable: true,
help: 'Treat warning level issues as fatal.',
defaultsTo: true);
} }
/// The working directory for testing analysis using dartanalyzer. /// The working directory for testing analysis using dartanalyzer.
......
...@@ -182,8 +182,7 @@ class AnalyzeOnce extends AnalyzeBase { ...@@ -182,8 +182,7 @@ class AnalyzeOnce extends AnalyzeBase {
if (errorCount > 0) { if (errorCount > 0) {
logger.printStatus(''); logger.printStatus('');
// We consider any level of error to be an error exit (we don't report different levels). throwToolExit(errorsMessage, exitCode: _isFatal(errors) ? 1 : 0);
throwToolExit(errorsMessage);
} }
if (argResults['congratulate'] as bool) { if (argResults['congratulate'] as bool) {
...@@ -194,4 +193,21 @@ class AnalyzeOnce extends AnalyzeBase { ...@@ -194,4 +193,21 @@ class AnalyzeOnce extends AnalyzeBase {
throwToolExit('Server error(s) occurred. (ran in ${seconds}s)'); throwToolExit('Server error(s) occurred. (ran in ${seconds}s)');
} }
} }
bool _isFatal(List<AnalysisError> errors) {
for (final AnalysisError error in errors) {
final AnalysisSeverity severityLevel = error.writtenError.severityLevel;
if (severityLevel == AnalysisSeverity.error) {
return true;
}
if (severityLevel == AnalysisSeverity.warning &&
(argResults['fatal-warnings'] as bool || argResults['fatal-infos'] as bool)) {
return true;
}
if (severityLevel == AnalysisSeverity.info && argResults['fatal-infos'] as bool) {
return true;
}
}
return false;
}
} }
...@@ -190,7 +190,7 @@ class AnalysisServer { ...@@ -190,7 +190,7 @@ class AnalysisServer {
} }
} }
enum _AnalysisSeverity { enum AnalysisSeverity {
error, error,
warning, warning,
info, info,
...@@ -217,12 +217,12 @@ class AnalysisError implements Comparable<AnalysisError> { ...@@ -217,12 +217,12 @@ class AnalysisError implements Comparable<AnalysisError> {
String get colorSeverity { String get colorSeverity {
switch (writtenError.severityLevel) { switch (writtenError.severityLevel) {
case _AnalysisSeverity.error: case AnalysisSeverity.error:
return _terminal.color(writtenError.severity, TerminalColor.red); return _terminal.color(writtenError.severity, TerminalColor.red);
case _AnalysisSeverity.warning: case AnalysisSeverity.warning:
return _terminal.color(writtenError.severity, TerminalColor.yellow); return _terminal.color(writtenError.severity, TerminalColor.yellow);
case _AnalysisSeverity.info: case AnalysisSeverity.info:
case _AnalysisSeverity.none: case AnalysisSeverity.none:
return writtenError.severity; return writtenError.severity;
} }
return null; return null;
...@@ -316,14 +316,14 @@ class WrittenError { ...@@ -316,14 +316,14 @@ class WrittenError {
final int startColumn; final int startColumn;
final int offset; final int offset;
static final Map<String, _AnalysisSeverity> _severityMap = <String, _AnalysisSeverity>{ static final Map<String, AnalysisSeverity> _severityMap = <String, AnalysisSeverity>{
'INFO': _AnalysisSeverity.info, 'INFO': AnalysisSeverity.info,
'WARNING': _AnalysisSeverity.warning, 'WARNING': AnalysisSeverity.warning,
'ERROR': _AnalysisSeverity.error, 'ERROR': AnalysisSeverity.error,
}; };
_AnalysisSeverity get severityLevel => AnalysisSeverity get severityLevel =>
_severityMap[severity] ?? _AnalysisSeverity.none; _severityMap[severity] ?? AnalysisSeverity.none;
String get messageSentenceFragment { String get messageSentenceFragment {
if (message.endsWith('.')) { if (message.endsWith('.')) {
......
...@@ -41,6 +41,7 @@ void main() { ...@@ -41,6 +41,7 @@ void main() {
List<String> errorTextContains, List<String> errorTextContains,
bool toolExit = false, bool toolExit = false,
String exitMessageContains, String exitMessageContains,
int exitCode = 0,
}) async { }) async {
try { try {
arguments.insert(0, '--flutter-root=${Cache.flutterRoot}'); arguments.insert(0, '--flutter-root=${Cache.flutterRoot}');
...@@ -53,6 +54,8 @@ void main() { ...@@ -53,6 +54,8 @@ void main() {
} }
if (exitMessageContains != null) { if (exitMessageContains != null) {
expect(e.message, contains(exitMessageContains)); expect(e.message, contains(exitMessageContains));
// May not analyzer exception the `exitCode` is `null`.
expect(e.exitCode ?? 0, exitCode);
} }
} }
assertContains(logger.statusText, statusTextContains); assertContains(logger.statusText, statusTextContains);
...@@ -194,6 +197,7 @@ flutter_project:lib/ ...@@ -194,6 +197,7 @@ flutter_project:lib/
], ],
exitMessageContains: '3 issues found.', exitMessageContains: '3 issues found.',
toolExit: true, toolExit: true,
exitCode: 1,
); );
}); });
...@@ -239,6 +243,7 @@ flutter_project:lib/ ...@@ -239,6 +243,7 @@ flutter_project:lib/
], ],
exitMessageContains: '2 issues found.', exitMessageContains: '2 issues found.',
toolExit: true, toolExit: true,
exitCode: 1,
); );
} finally { } finally {
if (optionsFile.existsSync()) { if (optionsFile.existsSync()) {
...@@ -284,6 +289,7 @@ void bar() { ...@@ -284,6 +289,7 @@ void bar() {
], ],
exitMessageContains: '1 issue found.', exitMessageContains: '1 issue found.',
toolExit: true, toolExit: true,
exitCode: 1
); );
} finally { } finally {
tryToDelete(tempDir); tryToDelete(tempDir);
...@@ -371,12 +377,157 @@ StringBuffer bar = StringBuffer('baz'); ...@@ -371,12 +377,157 @@ StringBuffer bar = StringBuffer('baz');
tryToDelete(tempDir); tryToDelete(tempDir);
} }
}); });
testUsingContext('analyze once with default options has info issue finally exit code 1.', () async {
final Directory tempDir = fileSystem.systemTempDirectory.createTempSync(
'flutter_analyze_once_default_options_info_issue_exit_code_1.');
_createDotPackages(tempDir.path);
const String infoSourceCode = '''
int analyze() {}
''';
tempDir.childFile('main.dart').writeAsStringSync(infoSourceCode);
try {
await runCommand(
command: AnalyzeCommand(
workingDirectory: fileSystem.directory(tempDir),
platform: _kNoColorTerminalPlatform,
terminal: terminal,
processManager: processManager,
logger: logger,
fileSystem: fileSystem,
artifacts: artifacts,
),
arguments: <String>['analyze', '--no-pub'],
statusTextContains: <String>[
'info',
'missing_return',
],
exitMessageContains: '1 issue found.',
toolExit: true,
exitCode: 1,
);
} finally {
tryToDelete(tempDir);
}
});
testUsingContext('analyze once with no-fatal-infos has info issue finally exit code 0.', () async {
final Directory tempDir = fileSystem.systemTempDirectory.createTempSync(
'flutter_analyze_once_no_fatal_infos_info_issue_exit_code_0.');
_createDotPackages(tempDir.path);
const String infoSourceCode = '''
int analyze() {}
''';
tempDir.childFile('main.dart').writeAsStringSync(infoSourceCode);
try {
await runCommand(
command: AnalyzeCommand(
workingDirectory: fileSystem.directory(tempDir),
platform: _kNoColorTerminalPlatform,
terminal: terminal,
processManager: processManager,
logger: logger,
fileSystem: fileSystem,
artifacts: artifacts,
),
arguments: <String>['analyze', '--no-pub', '--no-fatal-infos'],
statusTextContains: <String>[
'info',
'missing_return',
],
exitMessageContains: '1 issue found.',
toolExit: true,
exitCode: 0,
);
} finally {
tryToDelete(tempDir);
}
});
testUsingContext('analyze once only fatal-warnings has info issue finally exit code 0.', () async {
final Directory tempDir = fileSystem.systemTempDirectory.createTempSync(
'flutter_analyze_once_only_fatal_warnings_info_issue_exit_code_0.');
_createDotPackages(tempDir.path);
const String infoSourceCode = '''
int analyze() {}
''';
tempDir.childFile('main.dart').writeAsStringSync(infoSourceCode);
try {
await runCommand(
command: AnalyzeCommand(
workingDirectory: fileSystem.directory(tempDir),
platform: _kNoColorTerminalPlatform,
terminal: terminal,
processManager: processManager,
logger: logger,
fileSystem: fileSystem,
artifacts: artifacts,
),
arguments: <String>['analyze', '--no-pub', '--fatal-warnings', '--no-fatal-infos'],
statusTextContains: <String>[
'info',
'missing_return',
],
exitMessageContains: '1 issue found.',
toolExit: true,
exitCode: 0,
);
} finally {
tryToDelete(tempDir);
}
});
testUsingContext('analyze once only fatal-infos has warning issue finally exit code 1.', () async {
final Directory tempDir = fileSystem.systemTempDirectory.createTempSync(
'flutter_analyze_once_only_fatal_infos_warning_issue_exit_code_1.');
_createDotPackages(tempDir.path);
const String warningSourceCode = '''
int analyze() {}
''';
final File optionsFile = fileSystem.file(fileSystem.path.join(tempDir.path, 'analysis_options.yaml'));
optionsFile.writeAsStringSync('''
analyzer:
errors:
missing_return: warning
''');
tempDir.childFile('main.dart').writeAsStringSync(warningSourceCode);
try {
await runCommand(
command: AnalyzeCommand(
workingDirectory: fileSystem.directory(tempDir),
platform: _kNoColorTerminalPlatform,
terminal: terminal,
processManager: processManager,
logger: logger,
fileSystem: fileSystem,
artifacts: artifacts,
),
arguments: <String>['analyze','--no-pub', '--fatal-infos', '--no-fatal-warnings'],
statusTextContains: <String>[
'warning',
'missing_return',
],
exitMessageContains: '1 issue found.',
toolExit: true,
exitCode: 1,
);
} finally {
tryToDelete(tempDir);
}
});
} }
void assertContains(String text, List<String> patterns) { void assertContains(String text, List<String> patterns) {
if (patterns == null) { if (patterns != null) {
expect(text, isEmpty);
} else {
for (final String pattern in patterns) { for (final String pattern in patterns) {
expect(text, contains(pattern)); expect(text, contains(pattern));
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment