Refactor stripFilePaths to better detect OK paths (#729)

lib/src/analysis_server.dart

@@ -284,14 +284,14 @@ abstract class AnalysisServerWrapper {
         final issue = proto.AnalysisIssue()
           ..kind = error.severity.toLowerCase()
           ..line = error.location.startLine
-          ..message = utils.stripFilePaths(error.message)
+          ..message = utils.normalizeFilePaths(error.message)
           ..sourceName = path.basename(error.location.file)
           ..hasFixes = error.hasFix
           ..charStart = error.location.offset
           ..charLength = error.location.length
           ..diagnosticMessages.addAll(error.contextMessages?.map((m) =>
                   proto.DiagnosticMessage(
-                      message: utils.stripFilePaths(m.message),
+                      message: utils.normalizeFilePaths(m.message),
                       line: m.location.startLine,
                       charStart: m.location.offset,
                       charLength: m.location.length)) ??
@@ -302,7 +302,7 @@ abstract class AnalysisServerWrapper {
         }
 
         if (error.correction != null) {
-          issue.correction = utils.stripFilePaths(error.correction);
+          issue.correction = utils.normalizeFilePaths(error.correction);
         }
 
         return issue;

lib/src/utils.dart

@@ -1,50 +1,48 @@
+// Copyright (c) 2021, 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:path/path.dart' as path;
 
-/// Matches any path in the string and replaces the part of the path before the
-/// last separator with either dart:core, package:flutter, or removes it.
+/// Normalizes any "paths" from [text], replacing the segments before the last
+/// separator with either "dart:core" or "package:flutter", or removes them,
+/// according to their content.
 ///
 /// ## Examples:
 ///
 /// "Unused import: '/path/foo.dart'" -> "Unused import: 'foo.dart'"
 ///
-/// "Unused import: '/path/to/dart/lib/world.dart'" -> "Unused import:
-/// 'dart:core/world.dart'"
+/// "Unused import: '/path/to/dart/lib/core/world.dart'" ->
+/// "Unused import: 'dart:core/world.dart'"
 ///
-/// "Unused import: 'package:flutter/material.dart'" -> "Unused import:
-/// 'package:flutter/material.dart'"
-String stripFilePaths(String str) {
-  // Match any URI. Also match URIs that are prefixed with dart:core or
-  // package:*
-  final regex = RegExp(r'(?:dart:core?)?(?:package:?)?[a-z]*\/\S*');
-
-  return str.replaceAllMapped(regex, (match) {
-    final urlString = match.group(0);
-    final pathComponents = path.split(urlString);
-    final isDartPath =
-        pathComponents.contains('lib') && pathComponents.contains('core');
-
-    // matches the 'flutter' package in the SDK
-    final isFlutterPath = pathComponents.contains('flutter');
-
-    final isPackagePath = urlString.contains('package:');
-    final isDartCorePath = urlString.contains('dart:core');
-    final basename = path.basename(urlString);
-
-    if (isFlutterPath) {
-      return path.join('package:flutter', basename);
+/// "Unused import: 'package:flutter/material.dart'" ->
+/// "Unused import: 'package:flutter/material.dart'"
+String normalizeFilePaths(String text) {
+  return text.replaceAllMapped(_possiblePathPattern, (match) {
+    final possiblePath = match.group(0);
+
+    final uri = Uri.tryParse(possiblePath);
+    if (uri != null && uri.hasScheme) {
+      return possiblePath;
     }
 
-    if (isDartCorePath) {
-      return urlString;
+    final pathComponents = path.split(possiblePath);
+    final basename = path.basename(possiblePath);
+
+    if (pathComponents.contains('flutter')) {
+      return path.join('package:flutter', basename);
     }
 
-    if (isDartPath) {
+    if (pathComponents.contains('lib') && pathComponents.contains('core')) {
       return path.join('dart:core', basename);
     }
 
-    if (isPackagePath) {
-      return urlString;
-    }
     return basename;
   });
 }
+
+/// A pattern which matches a possible path.
+///
+/// This pattern is essentially "possibly some letters and colons, followed by a
+/// slash, followed by non-whitespace."
+final _possiblePathPattern = RegExp(r'[a-zA-Z:]*\/\S*');

pubspec.lock

@@ -176,13 +176,6 @@ packages:
       url: "https://pub.dartlang.org"
     source: hosted
     version: "0.5.0"
-  expected_output:
-    dependency: "direct dev"
-    description:
-      name: expected_output
-      url: "https://pub.dartlang.org"
-    source: hosted
-    version: "2.0.0"
   file:
     dependency: transitive
     description:

pubspec.yaml

@@ -26,7 +26,6 @@ dev_dependencies:
   async: ^2.2.0
   build_runner: ^1.10.6
   coverage: ^0.15.2
-  expected_output: ^2.0.0
   grinder: ^0.9.0
   lints: ^1.0.1
   mock_request: ^1.0.7

test/utils_test.dart

@@ -1,16 +1,61 @@
-library utils.tests;
+// Copyright (c) 2021, 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:dart_services/src/utils.dart';
-import 'package:expected_output/expected_output.dart';
 import 'package:test/test.dart';
 
 void main() {
-  group('utils.stripFilePaths', () {
-    for (final dataCase in dataCasesUnder(library: #utils.tests)) {
-      test(dataCase.testDescription, () {
-        final actualOutput = stripFilePaths(dataCase.input);
-        expect(actualOutput, equals(dataCase.expectedOutput));
-      });
-    }
+  void expectNormalizeFilePaths(String input, String output) {
+    expect(normalizeFilePaths(input), equals(output));
+  }
+
+  test('normalizeFilePaths strips a temporary directory path', () {
+    expectNormalizeFilePaths(
+      'List is defined in /var/folders/4p/y54w9nqj0_n6ryqwn7lxqz6800m6cw/T/DartAnalysisWrapperintLAw/main.dart',
+      'List is defined in main.dart',
+    );
+  });
+
+  test('normalizeFilePaths replaces a SDK path with "dart:core"', () {
+    expectNormalizeFilePaths(
+      'List is defined in /path/dart/dart/sdk/lib/core/list.dart',
+      'List is defined in dart:core/list.dart',
+    );
+  });
+
+  test('normalizeFilePaths replaces a specific SDK path with "dart:core"', () {
+    expectNormalizeFilePaths(
+      "The argument type 'List<int> (where List is defined in /Users/username/sdk/dart/2.10.5/lib/core/list.dart)' can't be assigned to the parameter type 'List<int> (where List is defined in /var/folders/4p/tmp/T/DartAnalysisWrapperintLAw/main.dart)'.",
+      "The argument type 'List<int> (where List is defined in dart:core/list.dart)' can't be assigned to the parameter type 'List<int> (where List is defined in main.dart)'.",
+    );
+  });
+
+  test('normalizeFilePaths keeps a "package:" path intact', () {
+    expectNormalizeFilePaths(
+      "Unused import: 'package:flutter/material.dart'.",
+      "Unused import: 'package:flutter/material.dart'.",
+    );
+  });
+
+  test('normalizeFilePaths keeps a "dart:core" path intact', () {
+    expectNormalizeFilePaths(
+      'dart:core/foo.dart',
+      'dart:core/foo.dart',
+    );
+  });
+
+  test('normalizeFilePaths keeps a web URL intact', () {
+    expectNormalizeFilePaths(
+      'See http://dart.dev/go/non-promo-property',
+      'See http://dart.dev/go/non-promo-property',
+    );
+  });
+
+  test('normalizeFilePaths strips a Flutter SDK path', () {
+    expectNormalizeFilePaths(
+      "The argument type 'StatelessWidget (where StatelessWidget is defined in /Users/username/path/to/dart-services/project_templates/flutter_project/main.dart)' can't be assigned to the parameter type 'StatelessWidget (where StatelessWidget is defined in /Users/username/path/to/dart-services/flutter-sdk/packages/flutter/lib/src/widgets/framework.dart)'.",
+      "The argument type 'StatelessWidget (where StatelessWidget is defined in main.dart)' can't be assigned to the parameter type 'StatelessWidget (where StatelessWidget is defined in package:flutter/framework.dart)'.",
+    );
   });
 }