Reland "Add support for recording context information with analysis errors, with one example"
This reverts commit b9bf239c6549f7a8bbe435d748576b9d00b1ba94.
Reason for revert: The angular and angular plugins have been updated so as to make this safe.
Original change's description:
> Revert "Add support for recording context information with analysis errors, with one example"
>
> This reverts commit 6114435ea5e70eb08a79ff4d432827f5d43991aa.
>
> Reason for revert: Broke angular and angular_analyzer_plugin
>
> Original change's description:
> > Add support for recording context information with analysis errors, with one example
> >
> > Change-Id: Iba1c48163ce264d85a68bcb9f70e5025a7cdbfbb
> > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/100563
> > Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
> > Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
>
> TBR=scheglov@google.com,brianwilkerson@google.com
>
> Change-Id: I275c6e4473de73199ea9b5e490379ba58aec1d43
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/100628
> Reviewed-by: Paul Berry <paulberry@google.com>
> Commit-Queue: Paul Berry <paulberry@google.com>
TBR=paulberry@google.com,scheglov@google.com,brianwilkerson@google.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Change-Id: Ib1b7e63a459e63bcbf02f26bcf167ddad45295b7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/101004
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/lib/src/services/completion/statement/statement_completion.dart b/pkg/analysis_server/lib/src/services/completion/statement/statement_completion.dart
index 6c73922..cf2f06f 100644
--- a/pkg/analysis_server/lib/src/services/completion/statement/statement_completion.dart
+++ b/pkg/analysis_server/lib/src/services/completion/statement/statement_completion.dart
@@ -324,12 +324,6 @@
}
_removeError(ParserErrorCode.EXPECTED_TOKEN, partialMatch: "']'");
_removeError(ScannerErrorCode.EXPECTED_TOKEN, partialMatch: "']'");
- var ms =
- _findError(ParserErrorCode.EXPECTED_TOKEN, partialMatch: "';'");
- if (ms != null) {
- // Ensure the semicolon gets inserted in the correct location.
- ms.offset = loc - 1;
- }
}
}
}
diff --git a/pkg/analysis_server/test/protocol_server_test.dart b/pkg/analysis_server/test/protocol_server_test.dart
index 48bac74..7bc7663 100644
--- a/pkg/analysis_server/test/protocol_server_test.dart
+++ b/pkg/analysis_server/test/protocol_server_test.dart
@@ -4,16 +4,17 @@
import 'dart:mirrors';
-import 'package:analysis_server/src/protocol_server.dart';
+import 'package:analysis_server/src/protocol_server.dart'
+ hide DiagnosticMessage;
import 'package:analysis_server/src/services/search/search_engine.dart';
import 'package:analyzer/dart/ast/ast.dart' as engine;
import 'package:analyzer/dart/element/element.dart' as engine;
import 'package:analyzer/dart/element/type.dart' as engine;
+import 'package:analyzer/diagnostic/diagnostic.dart';
import 'package:analyzer/error/error.dart' as engine;
import 'package:analyzer/src/dart/error/lint_codes.dart';
import 'package:analyzer/src/error/codes.dart' as engine;
import 'package:analyzer/src/generated/source.dart' as engine;
-import 'package:analyzer_plugin/protocol/protocol_common.dart';
import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
@@ -264,6 +265,18 @@
MockAnalysisError(
this.source, this.errorCode, this.offset, this.length, this.message);
+
+ @override
+ List<DiagnosticMessage> get contextMessages => null;
+
+ @override
+ String get correctionMessage => null;
+
+ @override
+ DiagnosticMessage get problemMessage => null;
+
+ @override
+ Severity get severity => null;
}
class MockErrorCode implements engine.ErrorCode {
diff --git a/pkg/analysis_server/test/services/linter/linter_test.dart b/pkg/analysis_server/test/services/linter/linter_test.dart
index 3965bba..14b0054 100644
--- a/pkg/analysis_server/test/services/linter/linter_test.dart
+++ b/pkg/analysis_server/test/services/linter/linter_test.dart
@@ -76,5 +76,8 @@
class _TestSource implements Source {
@override
+ String get fullName => '/package/lib/test.dart';
+
+ @override
noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
}
diff --git a/pkg/analyzer/lib/diagnostic/diagnostic.dart b/pkg/analyzer/lib/diagnostic/diagnostic.dart
new file mode 100644
index 0000000..43b099a
--- /dev/null
+++ b/pkg/analyzer/lib/diagnostic/diagnostic.dart
@@ -0,0 +1,49 @@
+// Copyright (c) 2019, 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.
+
+/// A diagnostic, as defined by the [Diagnostic Design Guidelines][guidelines]:
+///
+/// > An indication of a specific problem at a specific location within the
+/// > source code being processed by a development tool.
+///
+/// Clients may not extend, implement or mix-in this class.
+///
+/// [guidelines]: ../doc/diagnostics.md
+abstract class Diagnostic {
+ /// A list of messages that provide context for understanding the problem
+ /// being reported. The list will be empty if there are no such messages.
+ List<DiagnosticMessage> get contextMessages;
+
+ /// A description of how to fix the problem, or `null` if there is no such
+ /// description.
+ String get correctionMessage;
+
+ /// A message describing what is wrong and why.
+ DiagnosticMessage get problemMessage;
+
+ /// The severity associated with the diagnostic.
+ Severity get severity;
+}
+
+/// A single message associated with a [Diagnostic], consisting of the text of
+/// the message and the location associated with it.
+///
+/// Clients may not extend, implement or mix-in this class.
+abstract class DiagnosticMessage {
+ /// The absolute and normalized path of the file associated with this message.
+ String get filePath;
+
+ /// The length of the source range associated with this message.
+ int get length;
+
+ /// The text of the message.
+ String get message;
+
+ /// The zero-based offset from the start of the file to the beginning of the
+ /// source range associated with this message.
+ int get offset;
+}
+
+/// An indication of the severity of a [Diagnostic].
+enum Severity { error, warning, info }
diff --git a/pkg/analyzer/lib/error/error.dart b/pkg/analyzer/lib/error/error.dart
index 02b5923..59e0168 100644
--- a/pkg/analyzer/lib/error/error.dart
+++ b/pkg/analyzer/lib/error/error.dart
@@ -4,8 +4,10 @@
import 'dart:collection';
+import 'package:analyzer/diagnostic/diagnostic.dart';
import 'package:analyzer/error/listener.dart';
import 'package:analyzer/src/dart/scanner/scanner.dart' show ScannerErrorCode;
+import 'package:analyzer/src/diagnostic/diagnostic.dart';
import 'package:analyzer/src/error/codes.dart';
import 'package:analyzer/src/generated/java_core.dart';
import 'package:analyzer/src/generated/parser.dart' show ParserErrorCode;
@@ -750,7 +752,7 @@
*
* See [AnalysisErrorListener].
*/
-class AnalysisError {
+class AnalysisError implements Diagnostic {
/**
* An empty array of errors used when no errors are expected.
*/
@@ -788,10 +790,9 @@
*/
final ErrorCode errorCode;
- /**
- * The localized error message.
- */
- String _message;
+ DiagnosticMessage _problemMessage;
+
+ List<DiagnosticMessage> _contextMessages;
/**
* The correction to be displayed for this error, or `null` if there is no
@@ -805,43 +806,40 @@
final Source source;
/**
- * The character offset from the beginning of the source (zero based) where
- * the error occurred.
- */
- int offset = 0;
-
- /**
- * The number of characters from the offset to the end of the source which
- * encompasses the compilation error.
- */
- int length = 0;
-
- /**
- * A flag indicating whether this error can be shown to be a non-issue because
- * of the result of type propagation.
- */
- bool isStaticOnly = false;
-
- /**
* Initialize a newly created analysis error. The error is associated with the
* given [source] and is located at the given [offset] with the given
* [length]. The error will have the given [errorCode] and the list of
- * [arguments] will be used to complete the message.
+ * [arguments] will be used to complete the message and correction. If any
+ * [contextMessages] are provided, they will be recorded with the error.
*/
- AnalysisError(this.source, this.offset, this.length, this.errorCode,
- [List<Object> arguments]) {
- this._message = formatList(errorCode.message, arguments);
+ AnalysisError(this.source, int offset, int length, this.errorCode,
+ [List<Object> arguments, List<DiagnosticMessage> contextMessages]) {
+ String message = formatList(errorCode.message, arguments);
String correctionTemplate = errorCode.correction;
if (correctionTemplate != null) {
this._correction = formatList(correctionTemplate, arguments);
}
+ _problemMessage = new DiagnosticMessageImpl(
+ filePath: source?.fullName,
+ length: length,
+ message: message,
+ offset: offset);
+ _contextMessages = contextMessages;
}
/**
* Initialize a newly created analysis error with given values.
*/
- AnalysisError.forValues(this.source, this.offset, this.length, this.errorCode,
- this._message, this._correction);
+ AnalysisError.forValues(this.source, int offset, int length, this.errorCode,
+ String message, this._correction) {
+ _problemMessage = new DiagnosticMessageImpl(
+ filePath: source?.fullName,
+ length: length,
+ message: message,
+ offset: offset);
+ }
+
+ List<DiagnosticMessage> get contextMessages => _contextMessages ?? const [];
/**
* Return the template used to create the correction to be displayed for this
@@ -851,18 +849,70 @@
String get correction => _correction;
@override
+ String get correctionMessage => _correction;
+
+ @override
int get hashCode {
int hashCode = offset;
- hashCode ^= (_message != null) ? _message.hashCode : 0;
+ hashCode ^= (message != null) ? message.hashCode : 0;
hashCode ^= (source != null) ? source.hashCode : 0;
return hashCode;
}
/**
+ * Return `true` if this error can be shown to be a non-issue because of the
+ * result of type propagation.
+ */
+ @Deprecated(
+ 'Type propagation is no longer performed, so this will never be true')
+ bool get isStaticOnly => false;
+
+ @Deprecated(
+ 'Type propagation is no longer performed, so this can never be true')
+ void set isStaticOnly(bool value) {}
+
+ /**
+ * The number of characters from the offset to the end of the source which
+ * encompasses the compilation error.
+ */
+ int get length => _problemMessage.length;
+
+ /**
* Return the message to be displayed for this error. The message should
* indicate what is wrong and why it is wrong.
*/
- String get message => _message;
+ String get message => _problemMessage.message;
+
+ /**
+ * The character offset from the beginning of the source (zero based) where
+ * the error occurred.
+ */
+ int get offset => _problemMessage.offset;
+
+ /**
+ * The character offset from the beginning of the source (zero based) where
+ * the error occurred.
+ */
+ @Deprecated('Set the offset when the error is created')
+ set offset(int offset) {}
+
+ @override
+ DiagnosticMessage get problemMessage => _problemMessage;
+
+ @override
+ Severity get severity {
+ switch (errorCode.errorSeverity) {
+ case ErrorSeverity.ERROR:
+ return Severity.error;
+ case ErrorSeverity.WARNING:
+ return Severity.warning;
+ case ErrorSeverity.INFO:
+ return Severity.info;
+ default:
+ throw new StateError(
+ 'Invalid error severity: ${errorCode.errorSeverity}');
+ }
+ }
@override
bool operator ==(Object other) {
@@ -878,11 +928,8 @@
if (offset != other.offset || length != other.length) {
return false;
}
- if (isStaticOnly != other.isStaticOnly) {
- return false;
- }
// Deep checks.
- if (_message != other._message) {
+ if (message != other.message) {
return false;
}
if (source != other.source) {
@@ -904,7 +951,7 @@
buffer.write(offset + length - 1);
buffer.write("): ");
//buffer.write("(" + lineNumber + ":" + columnNumber + "): ");
- buffer.write(_message);
+ buffer.write(message);
return buffer.toString();
}
diff --git a/pkg/analyzer/lib/src/diagnostic/diagnostic.dart b/pkg/analyzer/lib/src/diagnostic/diagnostic.dart
new file mode 100644
index 0000000..d45ad279
--- /dev/null
+++ b/pkg/analyzer/lib/src/diagnostic/diagnostic.dart
@@ -0,0 +1,30 @@
+// Copyright (c) 2019, 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.
+
+import 'package:analyzer/diagnostic/diagnostic.dart';
+import 'package:meta/meta.dart';
+
+/// A concrete implementation of a diagnostic message.
+class DiagnosticMessageImpl implements DiagnosticMessage {
+ @override
+ final String filePath;
+
+ @override
+ final int length;
+
+ @override
+ final String message;
+
+ @override
+ final int offset;
+
+ /// Initialize a newly created message to represent a [message] reported in
+ /// the file at the given [filePath] at the given [offset] and with the given
+ /// [length].
+ DiagnosticMessageImpl(
+ {@required this.filePath,
+ @required this.length,
+ @required this.message,
+ @required this.offset});
+}
diff --git a/pkg/analyzer/lib/src/diagnostic/diagnostic_factory.dart b/pkg/analyzer/lib/src/diagnostic/diagnostic_factory.dart
new file mode 100644
index 0000000..7b359dd
--- /dev/null
+++ b/pkg/analyzer/lib/src/diagnostic/diagnostic_factory.dart
@@ -0,0 +1,49 @@
+// Copyright (c) 2019, 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.
+
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/diagnostic/diagnostic.dart';
+import 'package:analyzer/error/error.dart';
+import 'package:analyzer/source/line_info.dart';
+import 'package:analyzer/src/diagnostic/diagnostic.dart';
+import 'package:analyzer/src/error/codes.dart';
+import 'package:analyzer/src/generated/source.dart';
+
+/// A factory used to create diagnostics.
+class DiagnosticFactory {
+ /// Initialize a newly created diagnostic factory.
+ DiagnosticFactory();
+
+ /// Return a diagnostic indicating that the given [identifier] was referenced
+ /// before it was declared.
+ AnalysisError referencedBeforeDeclaration(
+ Source source, Identifier identifier,
+ {Element element}) {
+ String name = identifier.name;
+ Element staticElement = element ?? identifier.staticElement;
+ List<DiagnosticMessage> contextMessages;
+ int declarationOffset = staticElement.nameOffset;
+ if (declarationOffset >= 0 && staticElement != null) {
+ CompilationUnitElement unit = staticElement
+ .getAncestor((element) => element is CompilationUnitElement);
+ CharacterLocation location = unit.lineInfo.getLocation(declarationOffset);
+ contextMessages = [
+ new DiagnosticMessageImpl(
+ filePath: source.fullName,
+ message:
+ "The declaration of '$name' is on line ${location.lineNumber}.",
+ offset: declarationOffset,
+ length: staticElement.nameLength)
+ ];
+ }
+ return new AnalysisError(
+ source,
+ identifier.offset,
+ identifier.length,
+ CompileTimeErrorCode.REFERENCED_BEFORE_DECLARATION,
+ [name],
+ contextMessages);
+ }
+}
diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart
index fa6ce99..f1d4d06 100644
--- a/pkg/analyzer/lib/src/generated/error_verifier.dart
+++ b/pkg/analyzer/lib/src/generated/error_verifier.dart
@@ -21,6 +21,7 @@
import 'package:analyzer/src/dart/element/inheritance_manager2.dart';
import 'package:analyzer/src/dart/element/member.dart';
import 'package:analyzer/src/dart/element/type.dart';
+import 'package:analyzer/src/diagnostic/diagnostic_factory.dart';
import 'package:analyzer/src/error/codes.dart';
import 'package:analyzer/src/error/literal_element_verifier.dart';
import 'package:analyzer/src/error/pending_error.dart';
@@ -4994,10 +4995,8 @@
_hiddenElements != null &&
_hiddenElements.contains(node.staticElement) &&
node.parent is! CommentReference) {
- _errorReporter.reportErrorForNode(
- CompileTimeErrorCode.REFERENCED_BEFORE_DECLARATION,
- node,
- [node.name]);
+ _errorReporter.reportError(new DiagnosticFactory()
+ .referencedBeforeDeclaration(_errorReporter.source, node));
}
}
diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart
index bc9d8dc..94b83c9 100644
--- a/pkg/analyzer/lib/src/generated/resolver.dart
+++ b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -27,6 +27,7 @@
import 'package:analyzer/src/dart/element/type.dart';
import 'package:analyzer/src/dart/resolver/exit_detector.dart';
import 'package:analyzer/src/dart/resolver/scope.dart';
+import 'package:analyzer/src/diagnostic/diagnostic_factory.dart';
import 'package:analyzer/src/error/codes.dart';
import 'package:analyzer/src/generated/constant.dart';
import 'package:analyzer/src/generated/element_resolver.dart';
@@ -6594,8 +6595,8 @@
} else if (element is LocalVariableElement ||
(element is FunctionElement &&
element.enclosingElement is ExecutableElement)) {
- reportErrorForNode(CompileTimeErrorCode.REFERENCED_BEFORE_DECLARATION,
- typeName, [typeName.name]);
+ errorListener.onError(new DiagnosticFactory()
+ .referencedBeforeDeclaration(source, typeName, element: element));
} else {
reportErrorForNode(
StaticWarningCode.NOT_A_TYPE, typeName, [typeName.name]);
diff --git a/pkg/analyzer/lib/src/summary2/ast_resolver.dart b/pkg/analyzer/lib/src/summary2/ast_resolver.dart
index 12ef69d..bf3ac6d 100644
--- a/pkg/analyzer/lib/src/summary2/ast_resolver.dart
+++ b/pkg/analyzer/lib/src/summary2/ast_resolver.dart
@@ -66,5 +66,9 @@
}
class _FakeSource implements Source {
+ @override
+ String get fullName => '/package/lib/test.dart';
+
+ @override
noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
}
diff --git a/pkg/analyzer_cli/test/driver_test.dart b/pkg/analyzer_cli/test/driver_test.dart
index 054521d..fa1cd22 100644
--- a/pkg/analyzer_cli/test/driver_test.dart
+++ b/pkg/analyzer_cli/test/driver_test.dart
@@ -789,24 +789,6 @@
});
}
- test_pubspecYaml() async {
- await withTempDirAsync((tempDir) async {
- String filePath = path.join(tempDir, AnalysisEngine.PUBSPEC_YAML_FILE);
- new File(filePath).writeAsStringSync('''
-name: foo
-flutter:
- assets:
- doesNotExist.gif
-''');
- await drive(filePath);
- expect(
- bulletToDash(outSink),
- contains(
- "warning - The value of the 'asset' field is expected to be a list of relative file paths"));
- expect(exitCode, 0);
- });
- }
-
test_manifestFileChecks() async {
await withTempDirAsync((tempDir) async {
String filePath =
@@ -830,6 +812,24 @@
expect(exitCode, 0);
});
}
+
+ test_pubspecYaml() async {
+ await withTempDirAsync((tempDir) async {
+ String filePath = path.join(tempDir, AnalysisEngine.PUBSPEC_YAML_FILE);
+ new File(filePath).writeAsStringSync('''
+name: foo
+flutter:
+ assets:
+ doesNotExist.gif
+''');
+ await drive(filePath);
+ expect(
+ bulletToDash(outSink),
+ contains(
+ "warning - The value of the 'asset' field is expected to be a list of relative file paths"));
+ expect(exitCode, 0);
+ });
+ }
}
@reflectiveTest
@@ -965,5 +965,8 @@
TestSource();
@override
+ String get fullName => '/package/lib/test.dart';
+
+ @override
noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
}
diff --git a/pkg/analyzer_cli/test/mocks.dart b/pkg/analyzer_cli/test/mocks.dart
index 3d3ad59..ab4e74f 100644
--- a/pkg/analyzer_cli/test/mocks.dart
+++ b/pkg/analyzer_cli/test/mocks.dart
@@ -2,6 +2,7 @@
// 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.
+import 'package:analyzer/diagnostic/diagnostic.dart';
import 'package:analyzer/error/error.dart';
import 'package:analyzer/source/line_info.dart';
import 'package:analyzer/src/generated/engine.dart';
@@ -30,7 +31,19 @@
MockAnalysisError(this.source, this.errorCode, this.offset, this.message);
@override
+ List<DiagnosticMessage> get contextMessages => null;
+
+ @override
String get correction => null;
+
+ @override
+ String get correctionMessage => null;
+
+ @override
+ DiagnosticMessage get problemMessage => null;
+
+ @override
+ Severity get severity => null;
}
class MockAnalysisErrorInfo implements AnalysisErrorInfo {
