Merge pull request #15 from Workiva/clean-up-diagnostics

Clean up diagnostics code organization

over_react_analyzer_plugin/lib/src/assist/add_props.dart

@@ -2,7 +2,7 @@ import 'package:analyzer_plugin/protocol/protocol_generated.dart';
 import 'package:analyzer_plugin/utilities/assist/assist.dart';
 import 'package:over_react_analyzer_plugin/src/assist/contributor_base.dart';
 import 'package:over_react_analyzer_plugin/src/component_usage.dart';
-import 'package:over_react_analyzer_plugin/src/diagnostic/component_usage.dart';
+import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart';
 import 'package:over_react_analyzer_plugin/src/fluent_interface_util.dart';
 
 class AddPropsAssistContributor extends AssistContributorBase {

over_react_analyzer_plugin/lib/src/async_plugin_apis/diagnostic.dart

@@ -33,15 +33,19 @@
 import 'dart:async';
 
 import 'package:analyzer/dart/analysis/results.dart';
-import 'package:analyzer/error/error.dart';
+import 'package:analyzer_plugin/channel/channel.dart';
 import 'package:analyzer_plugin/plugin/plugin.dart';
+import 'package:analyzer_plugin/protocol/protocol.dart';
 import 'package:analyzer_plugin/protocol/protocol_common.dart' as plugin;
+import 'package:analyzer_plugin/protocol/protocol_common.dart';
 import 'package:analyzer_plugin/protocol/protocol_generated.dart' as plugin;
 import 'package:analyzer_plugin/protocol/protocol_generated.dart';
 // ignore: implementation_imports
 import 'package:analyzer_plugin/src/utilities/fixes/fixes.dart';
 import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
-import 'package:over_react_analyzer_plugin/src/diagnostic/component_usage.dart';
+import 'package:meta/meta.dart';
+import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart';
+import 'package:over_react_analyzer_plugin/src/error_filtering.dart';
 
 mixin DiagnosticMixin on ServerPlugin {
   List<DiagnosticContributor> getDiagnosticContributors(String path);
@@ -56,7 +60,7 @@ mixin DiagnosticMixin on ServerPlugin {
         // If there is something to analyze, do so and notify the analyzer.
         // Note that notifying with an empty set of errors is important as
         // this clears errors if they were fixed.
-        final generator = DiagnosticGenerator(getDiagnosticContributors(analysisResult.path));
+        final generator = _DiagnosticGenerator(getDiagnosticContributors(analysisResult.path));
         final result = await generator.generateErrors(analysisResult);
         channel.sendNotification(plugin.AnalysisErrorsParams(analysisResult.path, result.result).toNotification());
         result.sendNotifications(channel);
@@ -72,7 +76,7 @@ mixin DiagnosticMixin on ServerPlugin {
     // We want request errors to propagate if they throw
     final request = await _getFixesRequest(parameters);
     try {
-      final generator = DiagnosticGenerator(getDiagnosticContributors(parameters.file));
+      final generator = _DiagnosticGenerator(getDiagnosticContributors(parameters.file));
       final result = await generator.generateFixesResponse(request);
       result.sendNotifications(channel);
       return result.result;
@@ -103,3 +107,99 @@ mixin DiagnosticMixin on ServerPlugin {
 //    return result.errors;
 //  }
 }
+
+/// A class that generates errors and fixes for a set of [contributors] for
+/// a given result unit or fixes request.
+@sealed
+class _DiagnosticGenerator {
+  /// Initialize a newly created errors generator to use the given
+  /// [contributors].
+  _DiagnosticGenerator(this.contributors);
+
+  /// The contributors to be used to generate the errors.
+  final List<DiagnosticContributor> contributors;
+
+  /// Creates a 'analysis.errors' response for the the file specified
+  /// by the given [unitResult]. If any of the contributors throws an exception,
+  /// also create a non-fatal 'plugin.error' notification.
+  Future<_GeneratorResult<List<AnalysisError>>> generateErrors(ResolvedUnitResult unitResult) async {
+    return _generateErrors(unitResult, DiagnosticCollectorImpl(shouldComputeFixes: false));
+  }
+
+  /// Creates an 'edit.getFixes' response for the location in the file specified
+  /// by the given [request]. If any of the contributors throws an exception,
+  /// also create a non-fatal 'plugin.error' notification.
+  Future<_GeneratorResult<EditGetFixesResult>> generateFixesResponse(DartFixesRequest request) async {
+    // Recompute the errors and then emit the matching fixes
+
+    final collector = DiagnosticCollectorImpl(shouldComputeFixes: true);
+    final errorsResult = await _generateErrors(request.result, collector);
+    final notifications = [...errorsResult.notifications];
+
+    // Return any fixes that contain the given offset.
+    // TODO use request.errorsToFix instead?
+    final fixes = <AnalysisErrorFixes>[];
+    for (var i = 0; i < collector.errors.length; i++) {
+      final error = collector.errors[i];
+      final errorStart = error.location.offset;
+      final errorEnd = errorStart + error.location.length;
+
+      // `<=` because we do want the end to be inclusive (you should get
+      // the fix when your cursor is on the tail end of the error).
+      if (request.offset >= errorStart && request.offset <= errorEnd) {
+        fixes.add(AnalysisErrorFixes(
+          error,
+          fixes: [collector.fixes[i]],
+        ));
+      }
+    }
+
+    return _GeneratorResult(EditGetFixesResult(fixes), notifications);
+  }
+
+  Future<_GeneratorResult<List<AnalysisError>>> _generateErrors(
+      ResolvedUnitResult unitResult, DiagnosticCollectorImpl collector) async {
+    final notifications = <Notification>[];
+    for (final contributor in contributors) {
+      try {
+        await contributor.computeErrors(unitResult, collector);
+      } catch (exception, stackTrace) {
+        notifications.add(PluginErrorParams(false, exception.toString(), stackTrace.toString()).toNotification());
+      }
+    }
+
+    // The analyzer normally filters out errors with "ignore" comments,
+    // but it doesn't do it for plugin errors, so we need to do that here.
+    final lineInfo = unitResult.unit.lineInfo;
+    final filteredErrors =
+        filterIgnores(collector.errors, lineInfo, () => IgnoreInfo.calculateIgnores(unitResult.content, lineInfo));
+
+    return _GeneratorResult(filteredErrors, notifications);
+  }
+}
+
+/// The result produced by a generator.
+///
+/// Adapted from `GeneratorResult` in analyzer_plugin, but with less restrictive typing.
+@sealed
+class _GeneratorResult<T> {
+  /// The result to be sent to the server, or `null` if there is no response, as
+  /// when the generator is generating a notification.
+  final T result;
+
+  /// The notifications that should be sent to the server. The list will be empty
+  /// if there are no notifications.
+  final List<Notification> notifications;
+
+  /// Initialize a newly created generator result with the given [result] and
+  /// [notifications].
+  _GeneratorResult(this.result, this.notifications);
+
+  /// Use the given communications [channel] to send the notifications to the
+  /// server.
+  void sendNotifications(PluginCommunicationChannel channel) {
+    for (final notification in notifications) {
+      channel.sendNotification(notification);
+    }
+  }
+}

over_react_analyzer_plugin/lib/src/component_usage.dart

@@ -167,3 +167,55 @@ FluentComponentUsage identifyUsage(AstNode node) {
   }
   return null;
 }
+
+class ComponentUsageVisitor extends RecursiveAstVisitor<void> {
+  ComponentUsageVisitor(this.onComponent);
+
+  final void Function(FluentComponentUsage) onComponent;
+
+  @override
+  void visitFunctionExpressionInvocation(FunctionExpressionInvocation node) {
+    visitInvocationExpression(node);
+  }
+
+  @override
+  void visitMethodInvocation(MethodInvocation node) {
+    visitInvocationExpression(node);
+  }
+
+  void visitInvocationExpression(InvocationExpression node) {
+    var usage = getComponentUsage(node);
+    if (usage != null) {
+      onComponent(usage);
+    }
+
+    node.visitChildren(this);
+  }
+}
+
+//
+//class ComponentUsageElementVisitor extends RecursiveElementVisitor<void> {
+//  final _OnComponent onComponent;
+//
+//  ComponentUsageElementVisitor(this.onComponent);
+//
+//  @override
+//  void visitFunctionExpressionInvocation(FunctionExpressionInvocation node) {
+//    return visitInvocationExpression(node);
+//  }
+//
+//  @override
+//  void visitMethodInvocation(MethodInvocation node) {
+//    return visitInvocationExpression(node);
+//  }
+//
+//  void visitInvocationExpression(InvocationExpression node) {
+//    var usage = getComponentUsage(node);
+//    if (usage != null) {
+//      onComponent(usage);
+//    }
+//
+//    node.visitChildren(this);
+//    return null;
+//  }
+//}

over_react_analyzer_plugin/lib/src/diagnostic/analyzer_debug_helper.dart

@@ -1,6 +1,6 @@
 import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer_plugin/protocol/protocol_common.dart';
-import 'package:over_react_analyzer_plugin/src/diagnostic/component_usage.dart';
+import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart';
 
 /// Usage:
 ///     AnalyzerDebugHelper debug = new AnalyzerDebugHelper(result, collector);
@@ -9,7 +9,7 @@ class AnalyzerDebugHelper {
 
   ResolvedUnitResult result;
   DiagnosticCollector collector;
-  static const code = ErrorCode(
+  static const code = DiagnosticCode(
     'over_react_debug_analyzer_plugin_helper',
     "{0}",
     AnalysisErrorSeverity.INFO,

over_react_analyzer_plugin/lib/src/diagnostic/arrow_function_prop.dart

@@ -1,11 +1,11 @@
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer_plugin/protocol/protocol_common.dart';
 import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
-import 'package:over_react_analyzer_plugin/src/diagnostic/component_usage.dart';
+import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart';
 import 'package:over_react_analyzer_plugin/src/fluent_interface_util.dart';
 
 class ArrowFunctionPropCascadeDiagnostic extends ComponentUsageDiagnosticContributor {
-  static final code = ErrorCode(
+  static final code = DiagnosticCode(
       'over_react_cascaded_arrow_functions',
       'Unparenthesized arrow function values prevent subsequent cascades',
       AnalysisErrorSeverity.WARNING,

over_react_analyzer_plugin/lib/src/diagnostic/bool_prop_name_readability.dart

@@ -1,9 +1,9 @@
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
-import 'package:over_react_analyzer_plugin/src/diagnostic/component_usage.dart';
+import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart';
 
 class BoolPropNameReadabilityDiagnostic extends DiagnosticContributor {
-  static const code = ErrorCode(
+  static const code = DiagnosticCode(
     'over_react_bool_prop_name_readability',
     "'{0}.{1}' isn't an easily readable Boolean prop name. Try using a prefix like: {2}",
     AnalysisErrorSeverity.INFO,

over_react_analyzer_plugin/lib/src/diagnostic/dom_prop_types.dart

@@ -1,10 +1,10 @@
-import 'package:over_react_analyzer_plugin/src/diagnostic/component_usage.dart';
+import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart';
 import 'package:over_react_analyzer_plugin/src/fluent_interface_util.dart';
 
 /// A diagnostic that warns when an HTML attribute set on an OverReact `Dom` component builder is invalid
 /// based on the `<attribute>: [<allowed_html_elems>]` schema found within [allowedHtmlElementsForAttribute].
 class InvalidDomAttributeDiagnostic extends ComponentUsageDiagnosticContributor {
-  static const code = ErrorCode(
+  static const code = DiagnosticCode(
     'over_react_invalid_dom_attribute',
     "'{0}' isn't a valid HTML attribute prop for '{1}'. It may only be used on: {2}",
     AnalysisErrorSeverity.WARNING,

over_react_analyzer_plugin/lib/src/diagnostic/duplicate_prop_cascade.dart

@@ -1,10 +1,10 @@
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer_plugin/utilities/pair.dart';
-import 'package:over_react_analyzer_plugin/src/diagnostic/component_usage.dart';
+import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart';
 import 'package:over_react_analyzer_plugin/src/fluent_interface_util.dart';
 
 class DuplicatePropCascadeDiagnostic extends ComponentUsageDiagnosticContributor {
-  static final code = ErrorCode(
+  static final code = DiagnosticCode(
       'over_react_duplicate_prop_cascade',
       "Prop '{0}' is set more than once ({1} of {2}). This is most likely a typo.",
       AnalysisErrorSeverity.WARNING,

over_react_analyzer_plugin/lib/src/diagnostic/hashcode_as_key.dart

@@ -1,8 +1,8 @@
-import 'package:over_react_analyzer_plugin/src/diagnostic/component_usage.dart';
+import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart';
 import 'package:over_react_analyzer_plugin/src/fluent_interface_util.dart';
 
 class HashCodeAsKeyDiagnostic extends ComponentUsageDiagnosticContributor {
-  static final code = ErrorCode(
+  static final code = DiagnosticCode(
       'over_react_missing_casecade_parens',
       "React keys should not be derived from 'hashCode' since it is not unique",
       AnalysisErrorSeverity.WARNING,

over_react_analyzer_plugin/lib/src/diagnostic/invalid_child.dart

@@ -5,11 +5,11 @@ import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/src/generated/type_system.dart' show TypeSystem;
 import 'package:analyzer_plugin/protocol/protocol_common.dart';
 import 'package:meta/meta.dart';
-import 'package:over_react_analyzer_plugin/src/diagnostic/component_usage.dart';
+import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart';
 import 'package:over_react_analyzer_plugin/src/fluent_interface_util.dart';
 
 class InvalidChildDiagnostic extends ComponentUsageDiagnosticContributor {
-  static final code = ErrorCode(
+  static final code = DiagnosticCode(
       'over_react_invalid_child',
       "Invalid child type: '{0}'. Must be a ReactElement, Fragment, string, number, boolean, null, or an Iterable of those types.{1}",
       AnalysisErrorSeverity.WARNING,

over_react_analyzer_plugin/lib/src/diagnostic/missing_cascade_parens.dart

@@ -5,10 +5,10 @@ import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer_plugin/protocol/protocol_common.dart';
 import 'package:over_react_analyzer_plugin/src/diagnostic/analyzer_debug_helper.dart';
-import 'package:over_react_analyzer_plugin/src/diagnostic/component_usage.dart';
+import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart';
 
 class MissingCascadeParensDiagnostic extends DiagnosticContributor {
-  static final code = ErrorCode(
+  static final code = DiagnosticCode(
       'over_react_missing_cascade_parens',
       'Are you missing parentheses around the builder cascade?',
       AnalysisErrorSeverity.WARNING,

over_react_analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart

@@ -1,11 +1,11 @@
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
-import 'package:over_react_analyzer_plugin/src/diagnostic/component_usage.dart';
+import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart';
 import 'package:over_react_analyzer_plugin/src/fluent_interface_util.dart';
 
 class MissingRequiredPropDiagnostic extends ComponentUsageDiagnosticContributor {
-  static final code = ErrorCode('over_react_required_prop', 'The prop {0} is required.', AnalysisErrorSeverity.WARNING,
-      AnalysisErrorType.STATIC_WARNING);
+  static final code = DiagnosticCode('over_react_required_prop', 'The prop {0} is required.',
+      AnalysisErrorSeverity.WARNING, AnalysisErrorType.STATIC_WARNING);
 
   static final fixKind = FixKind(code.name, 200, 'Add required prop \'{0}\'');
 

over_react_analyzer_plugin/lib/src/diagnostic/pseudo_static_lifecycle.dart

@@ -1,7 +1,7 @@
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:analyzer/dart/element/element.dart';
-import 'package:over_react_analyzer_plugin/src/diagnostic/component_usage.dart';
+import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart';
 
 const staticMethodNames = ['getDefaultProps', 'defaultProps', 'getDerivedStateFromProps'];
 const instanceMemberWhitelist = [
@@ -14,7 +14,7 @@ const instanceMemberWhitelist = [
 ];
 
 class PseudoStaticLifecycleDiagnostic extends DiagnosticContributor {
-  static final code = ErrorCode(
+  static final code = DiagnosticCode(
       'over_react_pseudo_static_lifecycle',
       '\'{0}\' must be treated as a static method; only super-calls '
           'and props/state utility methods (like \'newProps\' and \'typedPropsFactory\') are allowed.',

over_react_analyzer_plugin/lib/src/diagnostic/render_return_value.dart

@@ -1,18 +1,18 @@
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:analyzer_plugin/protocol/protocol_common.dart';
-import 'package:over_react_analyzer_plugin/src/diagnostic/component_usage.dart';
+import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart';
 import 'package:over_react_analyzer_plugin/src/diagnostic/invalid_child.dart';
 import 'package:over_react_analyzer_plugin/src/fluent_interface_util.dart';
 
 class RenderReturnValueDiagnostic extends DiagnosticContributor {
-  static final invalidTypeErrorCode = ErrorCode(
+  static final invalidTypeErrorCode = DiagnosticCode(
       'over_react_invalid_render_return_type',
       "Invalid render() return type: '{0}'. Must be a ReactElement, primitive value, or an Iterable of those types.{1}",
       AnalysisErrorSeverity.WARNING,
       AnalysisErrorType.STATIC_TYPE_WARNING);
 
-  static final preferNullOverFalseErrorCode = ErrorCode(
+  static final preferNullOverFalseErrorCode = DiagnosticCode(
       'over_react_prefer_null_over_false',
       'Prefer returning null over false in render. (The dart2js bug involving null has been fixed.)',
       AnalysisErrorSeverity.WARNING,

over_react_analyzer_plugin/lib/src/diagnostic/string_ref.dart

@@ -1,8 +1,8 @@
-import 'package:over_react_analyzer_plugin/src/diagnostic/component_usage.dart';
+import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart';
 import 'package:over_react_analyzer_plugin/src/fluent_interface_util.dart';
 
 class StringRefDiagnostic extends ComponentUsageDiagnosticContributor {
-  static const code = ErrorCode(
+  static const code = DiagnosticCode(
     'over_react_string_ref',
     'String refs are deprecated. Use a callback ref instead.',
     // todo make error in Component2

over_react_analyzer_plugin/lib/src/diagnostic/style_missing_unit.dart

@@ -1,11 +1,11 @@
 import 'package:analyzer/analyzer.dart' show ConstantEvaluator;
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
-import 'package:over_react_analyzer_plugin/src/diagnostic/component_usage.dart';
+import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart';
 import 'package:over_react_analyzer_plugin/src/fluent_interface_util.dart';
 
 class StyleMissingUnitDiagnostic extends ComponentUsageDiagnosticContributor {
-  static final code = ErrorCode(
+  static final code = DiagnosticCode(
       'over_react_style_missing_unit',
       // TODO upgrade to error in React 16
       "React CSS values must be strings with units, or numbers (in which case 'px' will be used). This will break in React 16.",