Merge pull request #401 from Workiva/dart2.18-lints

FEDX-13: Dart 2.18 lints & formatting

.github/workflows/dart_ci.yml

@@ -17,52 +17,32 @@ jobs:
       matrix:
         sdk: [ 2.13.4, 2.18.7, stable, beta ]
     steps:
-      - uses: actions/checkout@v2
-      - uses: dart-lang/setup-dart@v1.3
+      - uses: actions/checkout@v3
+      - uses: dart-lang/setup-dart@v1
         with:
           sdk: ${{ matrix.sdk }}
-
-      - id: install
-        name: Install dependencies
-        run: dart pub get
-
-      - name: Validate dependencies
-        run: dart run dependency_validator
-        if: always() && steps.install.outcome == 'success'
-
-      - name: Analyze project source
-        run: dart run dart_dev analyze
-        if: always() && steps.install.outcome == 'success'
-
-      - name: Run tests
-        run: dart run dart_dev test
-        if: always() && steps.install.outcome == 'success'
+      - run: dart pub get
+      - run: dart run dependency_validator
+      - run: dart run dart_dev analyze
+      - run: dart run dart_dev test
 
   format_and_publish:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
-      - uses: dart-lang/setup-dart@v1.3
+      - uses: actions/checkout@v3
+      - uses: dart-lang/setup-dart@v1
         with:
-          sdk: 2.13.4
-
-      - id: install
-        name: Install dependencies
-        run: dart pub get
-
-      - name: Verify formatting
-        run: dart run dart_dev format --check
-        if: always() && steps.install.outcome == 'success'
-
-      - name: Publish dry run
-        run: dart pub publish --dry-run
-        if: always() && steps.install.outcome == 'success'
+          sdk: 2.18.7
+      - run: dart pub get
+      - run: dart run dart_dev format --check
+      # TODO: uncomment once https://github.com/dart-lang/sdk/issues/51398 is fixed
+      # - run: dart pub publish --dry-run
 
   windows:
     runs-on: windows-latest
     steps:
-      - uses: actions/checkout@v2
-      - uses: dart-lang/setup-dart@v1.3
+      - uses: actions/checkout@v3
+      - uses: dart-lang/setup-dart@v1
         with:
           sdk: stable
 

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## 3.9.3-dev
+
+- Fixes lints on Dart 2.18
+- Formatting changes on Dart 2.18
+- Replace deprecated pedantic package with lints package
+
 ## [3.9.2](https://github.com/Workiva/dart_dev/compare/3.9.1...3.9.2)
 
 - Automatically use `dart analyze` instead of `dartanalyzer` when on Dart SDK

analysis_options.yaml

@@ -1,4 +1,4 @@
-include: package:pedantic/analysis_options.1.8.0.yaml
+include: package:lints/recommended.yaml
 
 analyzer:
   exclude:
@@ -12,6 +12,9 @@ analyzer:
 
 linter:
   rules:
-    - avoid_types_on_closure_parameters
-    - prefer_void_to_null
-    - void_checks
+    include:
+      - avoid_types_on_closure_parameters
+      - prefer_void_to_null
+      - void_checks
+    exclude:
+      - overridden_fields

lib/src/events.dart

@@ -5,7 +5,8 @@ Future<void> commandComplete(CommandResult result) async {
       _commandCompleteListeners.map((listener) => listener(result)));
 }
 
-void onCommandComplete(FutureOr<dynamic> callback(CommandResult result)) {
+void onCommandComplete(
+    FutureOr<dynamic> Function(CommandResult result) callback) {
   _commandCompleteListeners.add(callback);
 }
 

lib/src/executable.dart

@@ -139,7 +139,9 @@ void main(List<String> args) async {
 }
 
 Future<void> runWithConfig(
-    List<String> args, _ConfigGetter configGetter) async {
+    // ignore: library_private_types_in_public_api
+    List<String> args,
+    _ConfigGetter configGetter) async {
   attachLoggerToStdio(args);
 
   try {

lib/src/tools/analyze_tool.dart

@@ -178,7 +178,7 @@ ProcessDeclaration buildProcess(
         context.argResults, context.usageException,
         commandName: context.commandName,
         usageFooter:
-            'Arguments can be passed to the "${analyzerUsed}" process via '
+            'Arguments can be passed to the "$analyzerUsed" process via '
             'the --analyzer-args option.');
   }
   var executable = useDartAnalyze ? exe.dart : exe.dartanalyzer;

lib/src/tools/compound_tool.dart

@@ -60,6 +60,7 @@ mixin CompoundToolMixin on DevTool {
   String get description => _description ??= _specs
       .map((s) => s.tool.description)
       .firstWhere((desc) => desc != null, orElse: () => null);
+  @override
   set description(String value) => _description = value;
   String _description;
 

lib/src/tools/format_tool.dart

@@ -9,7 +9,6 @@ import 'package:io/io.dart' show ExitCode;
 import 'package:logging/logging.dart';
 import 'package:path/path.dart' as p;
 
-import '../../utils.dart';
 import '../dart_dev_tool.dart';
 import '../utils/arg_results_utils.dart';
 import '../utils/assert_no_positional_args_nor_args_after_separator.dart';
@@ -179,7 +178,7 @@ class FormatTool extends DevTool {
     final dir = Directory(root ?? '.');
 
     // Use Glob.listSync to get all directories which might include a matching file.
-    var directoriesWithExcludes = Set<String>();
+    var directoriesWithExcludes = <String>{};
 
     if (collapseDirectories) {
       for (var g in exclude) {

lib/src/tools/test_tool.dart

@@ -214,7 +214,7 @@ List<String> buildArgs({
     ...?configuredTestArgs,
     // 2. The --reporter option.
     if (argResults?.wasParsed('reporter') ?? false)
-      '--reporter=' + singleOptionValue(argResults, 'reporter'),
+      '--reporter=${singleOptionValue(argResults, 'reporter')}',
     // 3. The -n|--name, -N|--plain-name, and -P|--preset options
     ...?multiOptionValue(argResults, 'name')?.map((v) => '--name=$v'),
     ...?multiOptionValue(argResults, 'plain-name')
@@ -328,7 +328,9 @@ Iterable<String> buildFiltersForTestArgs(List<String> testArgs) {
   final filters = <String>[];
   for (final input in testInputs) {
     if (input.endsWith('.dart')) {
-      filters..add('$input.*_test.dart.js*')..add(dartExtToHtml(input));
+      filters
+        ..add('$input.*_test.dart.js*')
+        ..add(dartExtToHtml(input));
     } else {
       filters.add('$input**');
     }

lib/src/tools/webdev_serve_tool.dart

@@ -61,6 +61,7 @@ class WebdevServeTool extends DevTool {
   // DevTool Overrides
   // ---------------------------------------------------------------------------
 
+  @override
   final ArgParser argParser = ArgParser()
     ..addFlag('release',
         abbr: 'r', help: 'Build with release mode defaults for builders.')
@@ -215,8 +216,8 @@ WebdevServeExecution buildExecution(
   if (!globalPackageIsActiveAndCompatible(
       'webdev', VersionConstraint.parse('^2.0.0'),
       environment: environment)) {
-    _log.severe(red.wrap(styleBold.wrap('webdev serve') +
-            ' could not run for this project.\n') +
+    _log.severe(red.wrap(
+            '${styleBold.wrap('webdev serve')} could not run for this project.\n') +
         yellow.wrap('You must have `webdev` globally activated:\n'
             '  dart pub global activate webdev ^2.0.0'));
     return WebdevServeExecution.exitEarly(ExitCode.config.code);

lib/src/utils/assert_no_positional_args_before_separator.dart

@@ -5,7 +5,7 @@ import 'has_any_positional_args_before_separator.dart';
 void assertNoPositionalArgs(
   String name,
   ArgResults argResults,
-  void usageException(String message), {
+  void Function(String message) usageException, {
   bool beforeSeparator,
 }) {
   beforeSeparator ??= false;

lib/src/utils/logging.dart

@@ -104,13 +104,9 @@ String humanReadable(Duration duration) {
 
 void logSubprocessHeader(Logger logger, String command, {Level level}) {
   level ??= Level.INFO;
-  logger.log(
-      level,
-      'Running subprocess:\n' +
-          magenta.wrap(command) +
-          '\n' +
-          '-' * (io.stdout.hasTerminal ? io.stdout.terminalColumns : 79) +
-          '\n');
+  final numColumns = io.stdout.hasTerminal ? io.stdout.terminalColumns : 79;
+  logger.log(level,
+      'Running subprocess:\n${magenta.wrap(command)}\n${'-' * numColumns}\n');
 }
 
 /// Logs an asynchronous [action] with [description] before and after.
@@ -119,7 +115,7 @@ void logSubprocessHeader(Logger logger, String command, {Level level}) {
 Future<T> logTimedAsync<T>(
   Logger logger,
   String description,
-  Future<T> action(), {
+  Future<T> Function() action, {
   Level level,
 }) async {
   level ??= Level.INFO;
@@ -138,7 +134,7 @@ Future<T> logTimedAsync<T>(
 T logTimedSync<T>(
   Logger logger,
   String description,
-  T action(), {
+  T Function() action, {
   Level level = Level.INFO,
 }) {
   final watch = Stopwatch()..start();

lib/src/utils/organize_directives/namespace_collector.dart

@@ -6,7 +6,7 @@ import 'package:analyzer/dart/ast/visitor.dart';
 /// Imports and exports are returned in the order they appear in a file when
 /// passed to `CompilationUnit.accept`.
 class NamespaceCollector extends SimpleAstVisitor<List<NamespaceDirective>> {
-  List<NamespaceDirective> _namespaces = [];
+  final List<NamespaceDirective> _namespaces = [];
 
   @override
   List<NamespaceDirective> visitExportDirective(ExportDirective node) {

lib/src/utils/organize_directives/organize_directives.dart

@@ -180,7 +180,9 @@ String _getSortedNamespaceString(
           namespaceWithQuotesReplaced,
         )
         .substring(namespace.start(), namespace.end());
-    sortedReplacement..write(source)..write('\n');
+    sortedReplacement
+      ..write(source)
+      ..write('\n');
   }
   return sortedReplacement.toString();
 }

lib/src/utils/organize_directives/organize_directives_in_paths.dart

@@ -16,7 +16,7 @@ int organizeDirectivesInPaths(
 
 /// Returns all file paths from a given set of files and directories.
 Iterable<String> _getAllFiles(Iterable<String> paths) {
-  final allFiles = Set<String>();
+  final allFiles = <String>{};
 
   for (final path in paths) {
     switch (FileSystemEntity.typeSync(path)) {

pubspec.yaml

@@ -23,6 +23,7 @@ dependencies:
 
 dev_dependencies:
   dependency_validator: ^3.0.0
+  lints: ^1.0.1
   matcher: ^0.12.5
   test: ^1.15.7
   test_descriptor: ^2.0.0

test/functional/format_tool_functional_test.dart

@@ -9,7 +9,7 @@ import '../functional.dart';
 
 void main() {
   group('Format Tool', () {
-    Future<_SourceFile> _format(String projectPath) async {
+    Future<_SourceFile> format(String projectPath) async {
       const filePath = 'lib/main.dart';
 
       final process = await runDevToolFunctionalTest('format', projectPath);
@@ -23,7 +23,7 @@ void main() {
     test('organize directives off', () async {
       const projectPath =
           'test/functional/fixtures/format/unsorted_imports/organize_directives_off/';
-      final sourceFile = await _format(projectPath);
+      final sourceFile = await format(projectPath);
       expect(
         sourceFile.contentsBefore,
         equals(sourceFile.contentsAfter),
@@ -33,7 +33,7 @@ void main() {
     test('organize directives on', () async {
       const projectPath =
           'test/functional/fixtures/format/unsorted_imports/organize_directives_on/';
-      final sourceFile = await _format(projectPath);
+      final sourceFile = await format(projectPath);
       expect(
         sourceFile.contentsBefore,
         isNot(equals(sourceFile.contentsAfter)),

test/tools/analyze_tool_test.dart

@@ -1,6 +1,4 @@
 @TestOn('vm')
-import 'dart:io';
-
 import 'package:args/args.dart';
 import 'package:args/command_runner.dart';
 import 'package:dart_dev/src/dart_dev_tool.dart';

test/tools/compound_tool_test.dart

@@ -45,7 +45,9 @@ void main() {
     });
 
     test('runs tools in order', () async {
-      final ct = CompoundTool()..addTool(tool())..addTool(tool());
+      final ct = CompoundTool()
+        ..addTool(tool())
+        ..addTool(tool());
       expect(await ct.run(), 0);
       expect(toolsRan, orderedEquals([0, 1]));
     });
@@ -73,7 +75,9 @@ void main() {
         return 0;
       }, argParser: ArgParser()..addFlag('bar'));
 
-      final ct = CompoundTool()..addTool(tool1)..addTool(tool2);
+      final ct = CompoundTool()
+        ..addTool(tool1)
+        ..addTool(tool2);
       await ct.run(DevToolExecutionContext(
           argResults: ct.argParser.parse(['--foo', '--bar', 'baz'])));
     });
@@ -130,7 +134,9 @@ void main() {
     test('parses all options', () {
       final parser1 = ArgParser()..addFlag('flag');
       final parser2 = ArgParser()..addOption('option');
-      final cap = CompoundArgParser()..addParser(parser1)..addParser(parser2);
+      final cap = CompoundArgParser()
+        ..addParser(parser1)
+        ..addParser(parser2);
       final args = ['--flag', '--option=foo', 'bar'];
       final argResults = cap.parse(args);
       expect(argResults['flag'], isTrue);
@@ -141,7 +147,9 @@ void main() {
     test('provides a combined usage output', () {
       final parser1 = ArgParser()..addFlag('flag');
       final parser2 = ArgParser()..addOption('option');
-      final cap = CompoundArgParser()..addParser(parser1)..addParser(parser2);
+      final cap = CompoundArgParser()
+        ..addParser(parser1)
+        ..addParser(parser2);
       expect(
           cap.usage, allOf(contains(parser1.usage), contains(parser2.usage)));
     });
@@ -178,7 +186,9 @@ void main() {
       final barParser = ArgParser()..addFlag('bar');
       final barTool = DevTool.fromFunction((_) => 0, argParser: barParser);
 
-      final compoundTool = CompoundTool()..addTool(fooTool)..addTool(barTool);
+      final compoundTool = CompoundTool()
+        ..addTool(fooTool)
+        ..addTool(barTool);
       final baseContext = DevToolExecutionContext(
           argResults: compoundTool.argParser.parse(['--foo', '--bar']));