Make RefactoringStatus a collection of generated RefactoringProblems.

pkg/analysis_server/lib/src/services/correction/status.dart

Index: pkg/analysis_server/lib/src/services/correction/status.dart
diff --git a/pkg/analysis_server/lib/src/services/correction/status.dart b/pkg/analysis_server/lib/src/services/correction/status.dart
index 1d38f3cb211937ab1b40ea94788ab85182595f1a..6adbf08feed71ee37f82962e4320a15a16508fec 100644
--- a/pkg/analysis_server/lib/src/services/correction/status.dart
+++ b/pkg/analysis_server/lib/src/services/correction/status.dart
@@ -4,16 +4,106 @@
 
 library services.status;
 
-import 'package:analysis_server/src/services/search/search_engine.dart';
+import 'package:analysis_server/src/protocol2.dart' hide Element;
 import 'package:analysis_server/src/services/correction/source_range.dart';
+import 'package:analysis_server/src/services/search/search_engine.dart';
 import 'package:analyzer/src/generated/ast.dart';
 import 'package:analyzer/src/generated/element.dart';
 import 'package:analyzer/src/generated/engine.dart';
-import 'package:analyzer/src/generated/java_core.dart';
 import 'package:analyzer/src/generated/source.dart';
 
 
 /**
+ * Creates a new [Location].
+ */
+Location createLocation(AnalysisContext context, Source source,

[Brian Wilkerson, 2014/08/22 14:24:04]

I'd like this better if these were factory methods on Location.

[scheglov, 2014/08/22 14:46:13]

Agree.
It's the next step.
I decided to do my first code generation in the next CL.

+    SourceRange range) {
+  int startLine = 0;
+  int startColumn = 0;
+  {
+    LineInfo lineInfo = context.getLineInfo(source);
+    if (lineInfo != null) {
+      LineInfo_Location offsetLocation = lineInfo.getLocation(range.offset);
+      startLine = offsetLocation.lineNumber;
+      startColumn = offsetLocation.columnNumber;
+    }
+  }
+  return new Location(
+      source.fullName,
+      range.offset,
+      range.length,
+      startLine,
+      startColumn);
+}
+
+
+/**
+ * Creates a new [Location] for the given [Element].
+ */
+Location createLocation_forElement(Element element) {
+  AnalysisContext context = element.context;
+  Source source = element.source;
+  SourceRange range = rangeElementName(element);
+  return createLocation(context, source, range);
+}
+
+
+/**
+ * Creates a new [Location] for the given [SearchMatch].
+ */
+Location createLocation_forMatch(SearchMatch match) {
+  Element enclosingElement = match.element;
+  return createLocation(
+      enclosingElement.context,
+      enclosingElement.source,
+      match.sourceRange);
+}
+
+
+/**
+ * Creates a new [Location] for the given [AstNode].
+ */
+Location createLocation_forNode(AstNode node) {
+  CompilationUnit unit = node.getAncestor((node) => node is CompilationUnit);
+  CompilationUnitElement unitElement = unit.element;
+  AnalysisContext context = unitElement.context;
+  Source source = unitElement.source;
+  SourceRange range = rangeNode(node);
+  return createLocation(context, source, range);
+}
+
+
+/**
+ * Creates a new [Location] for the given [CompilationUnit].
+ */
+Location createLocation_forUnit(CompilationUnit unit, SourceRange range) {
+  CompilationUnitElement unitElement = unit.element;
+  AnalysisContext context = unitElement.context;
+  Source source = unitElement.source;
+  return createLocation(context, source, range);
+}
+
+
+RefactoringProblemSeverity _maxSeverity(RefactoringProblemSeverity a,

[Brian Wilkerson, 2014/08/22 14:24:04]

And this should be an instance method on RefactoringProblemSeverity.

[scheglov, 2014/08/22 14:46:13]

Do you mean a *static* method?
The initial values of a severity is null, because we don't have yet a problem
and there is no OK severity.

+    RefactoringProblemSeverity b) {
+  if (a == null) {
+    return b;

[Brian Wilkerson, 2014/08/22 14:24:04]

For consistency we should return 'a' if 'b' is null.

[scheglov, 2014/08/22 14:46:13]

Done.

+  } else if (a == RefactoringProblemSeverity.INFO) {
+    return b;
+  } else if (a == RefactoringProblemSeverity.WARNING) {
+    if (b == RefactoringProblemSeverity.ERROR ||
+        b == RefactoringProblemSeverity.FATAL) {
+      return b;
+    }
+  } else if (a == RefactoringProblemSeverity.ERROR) {
+    if (b == RefactoringProblemSeverity.FATAL) {
+      return b;
+    }
+  }
+  return a;
+}
+
+/**
  * An outcome of a condition checking operation.
  */
 class RefactoringStatus {
@@ -21,12 +111,12 @@ class RefactoringStatus {
    * The current severity of this [RefactoringStatus] - the maximum of the
    * severities of its [entries].
    */
-  RefactoringStatusSeverity _severity = RefactoringStatusSeverity.OK;
+  RefactoringProblemSeverity _severity = null;
 
   /**
-   * A list of [RefactoringStatusEntry]s.
+   * A list of [RefactoringProblem]s.
    */
-  final List<RefactoringStatusEntry> entries = [];
+  final List<RefactoringProblem> problems = [];
 
   /**
    * Creates a new OK [RefactoringStatus].
@@ -36,103 +126,107 @@ class RefactoringStatus {
   /**
    * Creates a new [RefactoringStatus] with the ERROR severity.
    */
-  factory RefactoringStatus.error(String msg,
-      [RefactoringStatusContext context]) {
+  factory RefactoringStatus.error(String msg, [Location location]) {
     RefactoringStatus status = new RefactoringStatus();
-    status.addError(msg, context);
+    status.addError(msg, location);
     return status;
   }
 
   /**
    * Creates a new [RefactoringStatus] with the FATAL severity.
    */
-  factory RefactoringStatus.fatal(String msg,
-      [RefactoringStatusContext context]) {
+  factory RefactoringStatus.fatal(String msg, [Location location]) {
     RefactoringStatus status = new RefactoringStatus();
-    status.addFatalError(msg, context);
+    status.addFatalError(msg, location);
     return status;
   }
 
   /**
    * Creates a new [RefactoringStatus] with the WARNING severity.
    */
-  factory RefactoringStatus.warning(String msg,
-      [RefactoringStatusContext context]) {
+  factory RefactoringStatus.warning(String msg, [Location location]) {
     RefactoringStatus status = new RefactoringStatus();
-    status.addWarning(msg, context);
+    status.addWarning(msg, location);
     return status;
   }
 
   /**
-   * Returns the first [RefactoringStatusEntry] with the highest severity.
-   *
-   * If there is more than one entry with the highest severity then there is no
-   * guarantee as to which will be returned.
-   *
-   * Returns `null` if no entries.
-   */
-  RefactoringStatusEntry get entryWithHighestSeverity {
-    for (RefactoringStatusEntry entry in entries) {
-      if (entry.severity == _severity) {
-        return entry;
-      }
-    }
-    return null;
-  }
-
-  /**
    * Returns `true` if the severity is FATAL or ERROR.
    */
-  bool get hasError =>
-      _severity == RefactoringStatusSeverity.FATAL ||
-          _severity == RefactoringStatusSeverity.ERROR;
+  bool get hasError {
+    return _severity == RefactoringProblemSeverity.FATAL ||
+        _severity == RefactoringProblemSeverity.ERROR;
+  }
 
   /**
    * Returns `true` if the severity is FATAL.
    */
-  bool get hasFatalError => _severity == RefactoringStatusSeverity.FATAL;
+  bool get hasFatalError => _severity == RefactoringProblemSeverity.FATAL;
 
   /**
    * Returns `true` if the severity is WARNING.
    */
-  bool get hasWarning => _severity == RefactoringStatusSeverity.WARNING;
+  bool get hasWarning => _severity == RefactoringProblemSeverity.WARNING;
 
   /**
    * Return `true` if the severity is `OK`.
    */
-  bool get isOK => _severity == RefactoringStatusSeverity.OK;
+  bool get isOK => _severity == null;
 
   /**
-   * Returns the message of the [RefactoringStatusEntry] with highest severity;
-   * may be `null` if no entries.
+   * Returns the message of the [RefactoringProblem] with highest severity;
+   * may be `null` if no problems.
    */
   String get message {
-    RefactoringStatusEntry entry = entryWithHighestSeverity;
-    if (entry == null) {
+    RefactoringProblem problem = this.problem;
+    if (problem == null) {
       return null;
     }
-    return entry.message;
+    return problem.message;
+  }
+
+  /**
+   * Returns the first [RefactoringProblem] with the highest severity.
+   *
+   * If there is more than one problem with the highest severity then there is no
+   * guarantee as to which will be returned.

[Paul Berry, 2014/08/22 12:36:03]

If you don't want to make a guarantee as to which problem will be returned, then
it seems odd that the summary of the function (on line 189) says "Returns the
*first* [RefactoringProblem] with the highest severity."

[scheglov, 2014/08/22 14:46:13]

Acknowledged.

+   *
+   * Returns `null` if no entries.
+   */
+  RefactoringProblem get problem {
+    for (RefactoringProblem problem in problems) {
+      if (problem.severity == _severity) {
+        return problem;
+      }
+    }
+    return null;
   }
 
   /**
    * Returns the current severity of this [RefactoringStatus].
    */
-  RefactoringStatusSeverity get severity => _severity;
+  RefactoringProblemSeverity get severity => _severity;
 
   /**
-   * Adds an ERROR entry with the given message and status.
+   * Adds an ERROR problem with the given message and location.
    */
-  void addError(String msg, [RefactoringStatusContext context]) {
-    _addEntry(
-        new RefactoringStatusEntry(RefactoringStatusSeverity.ERROR, msg, context));
+  void addError(String msg, [Location location]) {
+    _addProblem(
+        new RefactoringProblem(
+            RefactoringProblemSeverity.ERROR,
+            msg,
+            location: location));
   }
 
   /**
-   * Adds a FATAL entry with the given message and status.
+   * Adds a FATAL problem with the given message and location.
    */
-  void addFatalError(String msg, [RefactoringStatusContext context]) {
-    _addEntry(
-        new RefactoringStatusEntry(RefactoringStatusSeverity.FATAL, msg, context));
+  void addFatalError(String msg, [Location location]) {
+    _addProblem(
+        new RefactoringProblem(
+            RefactoringProblemSeverity.FATAL,
+            msg,
+            location: location));
   }
 
   /**
@@ -148,45 +242,35 @@ class RefactoringStatus {
     if (other == null) {
       return;
     }
-    entries.addAll(other.entries);
-    _severity = RefactoringStatusSeverity._max(_severity, other.severity);
+    problems.addAll(other.problems);
+    _severity = _maxSeverity(_severity, other.severity);
   }
 
   /**
-   * Adds a WARNING entry with the given message and status.
+   * Adds a WARNING problem with the given message and location.
    */
-  void addWarning(String msg, [RefactoringStatusContext context]) {
-    _addEntry(
-        new RefactoringStatusEntry(RefactoringStatusSeverity.WARNING, msg, context));
-  }
-
-  /**
-   * Returns a copy of this [RefactoringStatus] with ERROR replaced with FATAL.
-   */
-  RefactoringStatus escalateErrorToFatal() {
-    RefactoringStatus result = new RefactoringStatus();
-    for (RefactoringStatusEntry entry in entries) {
-      if (entry.severity == RefactoringStatusSeverity.ERROR) {
-        entry = new RefactoringStatusEntry(
-            RefactoringStatusSeverity.FATAL,
-            entry.message,
-            entry.context);
-      }
-      result._addEntry(entry);
-    }
-    return result;
+  void addWarning(String msg, [Location location]) {
+    _addProblem(
+        new RefactoringProblem(
+            RefactoringProblemSeverity.WARNING,
+            msg,
+            location: location));
   }
 
   @override
   String toString() {
     StringBuffer sb = new StringBuffer();
     sb.write("<");
-    sb.write(_severity.name);
+    if (_severity == null) {
+      sb.write('OK');
+    } else {
+      sb.write(_severity.name);
+    }
     if (!isOK) {
       sb.write("\n");
-      for (RefactoringStatusEntry entry in entries) {
+      for (RefactoringProblem problem in problems) {
         sb.write("\t");
-        sb.write(entry);
+        sb.write(problem);
         sb.write("\n");
       }
     }
@@ -195,205 +279,12 @@ class RefactoringStatus {
   }
 
   /**
-   * Adds the given [RefactoringStatusEntry] and updates [severity].
-   */
-  void _addEntry(RefactoringStatusEntry entry) {
-    entries.add(entry);
-    _severity = RefactoringStatusSeverity._max(_severity, entry.severity);
-  }
-}
-
-
-/**
- * [RefactoringStatusContext] can be used to annotate [RefactoringStatusEntry]s
- * with additional information typically presented in the user interface.
- */
-class RefactoringStatusContext {
-  /**
-   * The [AnalysisContext] in which this status occurs.
-   */
-  final AnalysisContext context;
-
-  /**
-   * The [Source] in which this status occurs.
+   * Adds the given [RefactoringProblem] and updates [severity].
    */
-  final Source source;
-
-  /**
-   * The [SourceRange] with specific location where this status occurs.
-   */
-  final SourceRange range;
-
-  /**
-   * Creates a new [RefactoringStatusContext].
-   */
-  RefactoringStatusContext(this.context, this.source, this.range);
-
-  /**
-   * Creates a new [RefactoringStatusContext] for the given [Element].
-   */
-  factory RefactoringStatusContext.forElement(Element element) {
-    AnalysisContext context = element.context;
-    Source source = element.source;
-    SourceRange range = rangeElementName(element);
-    return new RefactoringStatusContext(context, source, range);
-  }
-
-  /**
-   * Creates a new [RefactoringStatusContext] for the given [SearchMatch].
-   */
-  factory RefactoringStatusContext.forMatch(SearchMatch match) {
-    Element enclosingElement = match.element;
-    return new RefactoringStatusContext(
-        enclosingElement.context,
-        enclosingElement.source,
-        match.sourceRange);
-  }
-
-  /**
-   * Creates a new [RefactoringStatusContext] for the given [AstNode].
-   */
-  factory RefactoringStatusContext.forNode(AstNode node) {
-    CompilationUnit unit = node.getAncestor((node) => node is CompilationUnit);
-    CompilationUnitElement unitElement = unit.element;
-    AnalysisContext context = unitElement.context;
-    Source source = unitElement.source;
-    SourceRange range = rangeNode(node);
-    return new RefactoringStatusContext(context, source, range);
-  }
-
-  /**
-   * Creates a new [RefactoringStatusContext] for the given [CompilationUnit].
-   */
-  factory RefactoringStatusContext.forUnit(CompilationUnit unit,
-      SourceRange range) {
-    CompilationUnitElement unitElement = unit.element;
-    AnalysisContext context = unitElement.context;
-    Source source = unitElement.source;
-    return new RefactoringStatusContext(context, source, range);
-  }
-
-  @override
-  String toString() {
-    JavaStringBuilder builder = new JavaStringBuilder();
-    builder.append("[source=");
-    builder.append(source);
-    builder.append(", range=");
-    builder.append(range);
-    builder.append("]");
-    return builder.toString();
-  }
-}
-
-
-/**
- * An immutable object representing an entry in a [RefactoringStatus].
- *
- * A [RefactoringStatusEntry] consists of a severity, a message and a context.
- */
-class RefactoringStatusEntry {
-  /**
-   * The severity level.
-   */
-  final RefactoringStatusSeverity severity;
-
-  /**
-   * The message of the status entry.
-   */
-  final String message;
-
-  /**
-   * The [RefactoringStatusContext] which can be used to show more detailed
-   * information regarding this status entry in the UI.
-   *
-   * May be `null` indicating that no context is available.
-   */
-  final RefactoringStatusContext context;
-
-  RefactoringStatusEntry(this.severity, this.message, [this.context]);
-
-  /**
-   * Returns whether the entry represents an error or not.
-   */
-  bool get isError => severity == RefactoringStatusSeverity.ERROR;
-
-  /**
-   * Returns whether the entry represents a fatal error or not.
-   */
-  bool get isFatalError => severity == RefactoringStatusSeverity.FATAL;
-
-  /**
-   * Returns whether the entry represents a warning or not.
-   */
-  bool get isWarning => severity == RefactoringStatusSeverity.WARNING;
-
-  @override
-  String toString() {
-    if (context != null) {
-      return "${severity}: ${message}; Context: ${context}";
-    } else {
-      return "${severity}: ${message}";
-    }
-  }
-}
-
-
-/**
- * Severity of [RefactoringStatus].
- */
-class RefactoringStatusSeverity {
-  /**
-   * The severity indicating the nominal case.
-   */
-  static const OK = const RefactoringStatusSeverity('OK', 0);
-
-  /**
-   * The severity indicating a warning.
-   *
-   * Use this severity if the refactoring can be performed, but you assume that
-   * the user could not be aware of problems or confusions resulting from the
-   * execution.
-   */
-  static const WARNING = const RefactoringStatusSeverity('WARNING', 2);
-
-  /**
-   * The severity indicating an error.
-   *
-   * Use this severity if the refactoring can be performed, but the refactoring
-   * will not be behavior preserving and/or the partial execution will lead to
-   * an inconsistent state (e.g. compile errors).
-   */
-  static const ERROR = const RefactoringStatusSeverity('ERROR', 3);
-
-  /**
-   * The severity indicating a fatal error.
-   *
-   * Use this severity if the refactoring cannot be performed, and execution
-   * would lead to major problems. Note that this completely blocks the user
-   * from performing this refactoring.
-   *
-   * It is often preferable to use an [ERROR] status and allow a partial
-   * execution (e.g. if just one reference to a refactored element cannot be
-   * updated).
-   */
-  static const FATAL = const RefactoringStatusSeverity('FATAL', 4);
-
-  final String name;
-  final int ordinal;
-
-  const RefactoringStatusSeverity(this.name, this.ordinal);
-
-  @override
-  String toString() => name;
-
-  /**
-   * Returns the most severe [RefactoringStatusSeverity].
-   */
-  static RefactoringStatusSeverity _max(RefactoringStatusSeverity a,
-      RefactoringStatusSeverity b) {
-    if (b.ordinal > a.ordinal) {
-      return b;
-    }
-    return a;
+  void _addProblem(RefactoringProblem problem) {
+    problems.add(problem);
+    // update maximum severity
+    RefactoringProblemSeverity severity = problem.severity;
+    _severity = _maxSeverity(_severity, severity);
   }
 }