Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to and fix latest lints #1335

Merged
merged 5 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
include: package:lints/recommended.yaml
linter:
rules:
- avoid_dynamic_calls
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all of these are in dart_flutter_team_lints

- directives_ordering
- unawaited_futures
include: package:dart_flutter_team_lints/analysis_options.yaml

analyzer:
errors:
comment_references: ignore
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOTS of failures here!

27 changes: 9 additions & 18 deletions benchmark/benchmark.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ const _formatsPerTrial = 30;
/// Note, these files use ".txt" because while they can be *parsed* correctly,
/// they don't resolve without error. That's OK because the formatter doesn't
/// care about that.
final source = loadFile('before.dart.txt');
final expected = loadFile('after.dart.txt');
final source = _loadFile('before.dart.txt');
final expected = _loadFile('after.dart.txt');

void main(List<String> args) {
var best = 99999999.0;
Expand All @@ -27,7 +27,7 @@ void main(List<String> args) {
// For a single benchmark, format the source multiple times.
String? result;
for (var j = 0; j < _formatsPerTrial; j++) {
result = formatSource();
result = _formatSource();
}

var elapsed =
Expand All @@ -47,32 +47,23 @@ void main(List<String> args) {
// Don't print the first run. It's always terrible since the VM hasn't
// warmed up yet.
if (i == 0) continue;
printResult("Run ${padLeft('#$i', 3)}", elapsed);
_printResult("Run ${'#$i'.padLeft(3)}", elapsed);
}

printResult('Best ', best);
_printResult('Best ', best);
}

String loadFile(String name) {
String _loadFile(String name) {
var path = p.join(p.dirname(p.fromUri(Platform.script)), name);
return File(path).readAsStringSync();
}

void printResult(String label, double time) {
print('$label: ${padLeft(time.toStringAsFixed(2), 4)}ms '
void _printResult(String label, double time) {
print('$label: ${time.toStringAsFixed(2).padLeft(4)}ms '
"${'=' * ((time * 5).toInt())}");
}

String padLeft(input, int length) {
var result = input.toString();
if (result.length < length) {
result = ' ' * (length - result.length) + result;
}

return result;
}

String formatSource() {
String _formatSource() {
var formatter = DartFormatter();
return formatter.format(source);
}
34 changes: 17 additions & 17 deletions bin/format.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,17 @@ void main(List<String> args) async {
usageError(parser, err.message);
}

if (argResults['help']) {
if (argResults['help'] as bool) {
printUsage(parser);
return;
}

if (argResults['version']) {
if (argResults['version'] as bool) {
print(dartStyleVersion);
return;
}

if (argResults['verbose'] && !(argResults['help'] as bool)) {
if (argResults['verbose'] as bool && !(argResults['help'] as bool)) {
usageError(parser, 'Can only use --verbose with --help.');
}

Expand All @@ -46,7 +46,7 @@ void main(List<String> args) async {
usageError(parser, exception.message);
}

if (argResults['dry-run'] && argResults['overwrite']) {
if (argResults['dry-run'] as bool && argResults['overwrite'] as bool) {
usageError(
parser, 'Cannot use --dry-run and --overwrite at the same time.');
}
Expand All @@ -61,13 +61,13 @@ void main(List<String> args) async {
var summary = Summary.none;
var output = Output.show;
var setExitIfChanged = false;
if (argResults['dry-run']) {
if (argResults['dry-run'] as bool) {
checkForReporterCollision('dry-run', 'overwrite');
checkForReporterCollision('dry-run', 'machine');

show = Show.dryRun;
output = Output.none;
} else if (argResults['overwrite']) {
} else if (argResults['overwrite'] as bool) {
checkForReporterCollision('overwrite', 'machine');

if (argResults.rest.isEmpty) {
Expand All @@ -77,17 +77,17 @@ void main(List<String> args) async {

show = Show.overwrite;
output = Output.write;
} else if (argResults['machine']) {
} else if (argResults['machine'] as bool) {
output = Output.json;
}

if (argResults['profile']) summary = Summary.profile();
if (argResults['profile'] as bool) summary = Summary.profile();

setExitIfChanged = argResults['set-exit-if-changed'];
setExitIfChanged = argResults['set-exit-if-changed'] as bool;

int pageWidth;
try {
pageWidth = int.parse(argResults['line-length']);
pageWidth = int.parse(argResults['line-length'] as String);
} on FormatException catch (_) {
usageError(
parser,
Expand All @@ -97,22 +97,22 @@ void main(List<String> args) async {

int indent;
try {
indent = int.parse(argResults['indent']);
if (indent < 0 || indent.toInt() != indent) throw FormatException();
indent = int.parse(argResults['indent'] as String);
if (indent < 0 || indent.toInt() != indent) throw const FormatException();
} on FormatException catch (_) {
usageError(
parser,
'--indent must be a non-negative integer, was '
'"${argResults['indent']}".');
}

var followLinks = argResults['follow-links'];
var followLinks = argResults['follow-links'] as bool;

var fixes = <StyleFix>[];
if (argResults['fix']) fixes.addAll(StyleFix.all);
if (argResults['fix'] as bool) fixes.addAll(StyleFix.all);
for (var fix in StyleFix.all) {
if (argResults['fix-${fix.name}']) {
if (argResults['fix']) {
if (argResults['fix-${fix.name}'] as bool) {
if (argResults['fix'] as bool) {
usageError(parser, '--fix-${fix.name} is redundant with --fix.');
}

Expand All @@ -133,7 +133,7 @@ void main(List<String> args) async {
output: output,
summary: summary,
setExitIfChanged: setExitIfChanged,
experimentFlags: argResults['enable-experiment']);
experimentFlags: argResults['enable-experiment'] as List<String>);

if (argResults.rest.isEmpty) {
await formatStdin(options, selection, argResults['stdin-name'] as String);
Expand Down
3 changes: 3 additions & 0 deletions example/format.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// Copyright (c) 2015, 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.

// ignore_for_file: unreachable_from_main
kevmoo marked this conversation as resolved.
Show resolved Hide resolved

import 'package:dart_style/dart_style.dart';
import 'package:dart_style/src/constants.dart';
import 'package:dart_style/src/debug.dart' as debug;
Expand Down
9 changes: 5 additions & 4 deletions lib/src/argument_list_visitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,11 @@ class ArgumentListVisitor {
this._allArguments,
this._arguments,
this._functions,
this._argumentsAfterFunctions) {
assert(_functions == null || _argumentsAfterFunctions != null,
'If _functions is passed, _argumentsAfterFunctions must be too.');
}
this._argumentsAfterFunctions)
: assert(
_functions == null || _argumentsAfterFunctions != null,
'If _functions is passed, _argumentsAfterFunctions must be too.',
kevmoo marked this conversation as resolved.
Show resolved Hide resolved
);

/// Builds chunks for the argument list.
void visit() {
Expand Down
16 changes: 8 additions & 8 deletions lib/src/cli/format_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class FormatCommand extends Command<int> {
Future<int> run() async {
var argResults = this.argResults!;

if (argResults['version']) {
if (argResults['version'] as bool) {
print(dartStyleVersion);
return 0;
}
Expand Down Expand Up @@ -78,7 +78,7 @@ class FormatCommand extends Command<int> {
}

// Can't use --verbose with anything but --help.
if (argResults['verbose'] && !(argResults['help'] as bool)) {
if (argResults['verbose'] as bool && !(argResults['help'] as bool)) {
usageException('Can only use --verbose with --help.');
}

Expand All @@ -87,11 +87,11 @@ class FormatCommand extends Command<int> {
usageException('Cannot print a summary with JSON output.');
}

var pageWidth = int.tryParse(argResults['line-length']) ??
var pageWidth = int.tryParse(argResults['line-length'] as String) ??
usageException('--line-length must be an integer, was '
'"${argResults['line-length']}".');

var indent = int.tryParse(argResults['indent']) ??
var indent = int.tryParse(argResults['indent'] as String) ??
usageException('--indent must be an integer, was '
'"${argResults['indent']}".');

Expand All @@ -101,10 +101,10 @@ class FormatCommand extends Command<int> {
}

var fixes = <StyleFix>[];
if (argResults['fix']) fixes.addAll(StyleFix.all);
if (argResults['fix'] as bool) fixes.addAll(StyleFix.all);
for (var fix in StyleFix.all) {
if (argResults['fix-${fix.name}']) {
if (argResults['fix']) {
if (argResults['fix-${fix.name}'] as bool) {
if (argResults['fix'] as bool) {
usageException('--fix-${fix.name} is redundant with --fix.');
}

Expand All @@ -119,7 +119,7 @@ class FormatCommand extends Command<int> {
usageException(exception.message);
}

var followLinks = argResults['follow-links'];
var followLinks = argResults['follow-links'] as bool;
var setExitIfChanged = argResults['set-exit-if-changed'] as bool;

var experimentFlags = argResults['enable-experiment'] as List<String>;
Expand Down
2 changes: 1 addition & 1 deletion lib/src/cli/options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ List<int>? parseSelection(ArgResults argResults, String optionName) {
try {
var coordinates = option.split(':');
if (coordinates.length != 2) {
throw FormatException(
throw const FormatException(
'Selection should be a colon-separated pair of integers, "123:45".');
}

Expand Down
10 changes: 5 additions & 5 deletions lib/src/cli/output.dart
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ enum Output {
break;

case Output.json:
// TODO(rnystrom): Put an empty selection in here to remain compatible with
// the old formatter. Since there's no way to pass a selection on the
// command line, this will never be used, which is why it's hard-coded to
// -1, -1. If we add support for passing in a selection, put the real
// result here.
// TODO(rnystrom): Put an empty selection in here to remain compatible
// with the old formatter. Since there's no way to pass a selection on
// the command line, this will never be used, which is why it's
// hard-coded to -1, -1. If we add support for passing in a selection,
// put the real result here.
print(jsonEncode({
'path': path,
'source': result.text,
Expand Down
7 changes: 4 additions & 3 deletions lib/src/dart_formatter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class DartFormatter {
///
/// If [uri] is given, it is a [String] or [Uri] used to identify the file
/// being formatted in error messages.
String format(String source, {uri}) {
String format(String source, {Object? uri}) {
if (uri == null) {
// Do nothing.
} else if (uri is Uri) {
Expand All @@ -82,8 +82,9 @@ class DartFormatter {
throw ArgumentError('uri must be `null`, a Uri, or a String.');
}

return formatSource(SourceCode(source, uri: uri, isCompilationUnit: true))
.text;
return formatSource(
SourceCode(source, uri: uri as String?, isCompilationUnit: true),
kevmoo marked this conversation as resolved.
Show resolved Hide resolved
).text;
}

/// Formats the given [source] string containing a single Dart statement.
Expand Down
10 changes: 5 additions & 5 deletions lib/src/debug.dart
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ final _none = _color('\u001b[0m');
final _bold = _color('\u001b[1m');

/// Prints [message] to stdout with each line correctly indented.
void log([message]) {
void log([Object? message]) {
if (message == null) {
print('');
return;
Expand All @@ -59,13 +59,13 @@ void log([message]) {
}

/// Wraps [message] in gray ANSI escape codes if enabled.
String gray(message) => '$_gray$message$_none';
String gray(Object message) => '$_gray$message$_none';

/// Wraps [message] in green ANSI escape codes if enabled.
String green(message) => '$_green$message$_none';
String green(Object message) => '$_green$message$_none';

/// Wraps [message] in bold ANSI escape codes if enabled.
String bold(message) => '$_bold$message$_none';
String bold(Object message) => '$_bold$message$_none';

/// Prints [chunks] to stdout, one chunk per line, with detailed information
/// about each chunk.
Expand Down Expand Up @@ -99,7 +99,7 @@ void dumpChunks(int start, List<Chunk> chunks) {
var row = <String>[];
row.add('$prefix$index:');

void writeIf(predicate, String Function() callback) {
void writeIf(bool predicate, String Function() callback) {
if (predicate) {
row.add(callback());
} else {
Expand Down
3 changes: 2 additions & 1 deletion lib/src/front_end/comment_writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ mixin CommentWriter {
// just override the script tag's line.
if (token.previous!.type == TokenType.SCRIPT_TAG) previousLine = tokenLine;

// ignore: prefer_const_constructors
kevmoo marked this conversation as resolved.
Show resolved Hide resolved
var comments = CommentSequence._([], []);
for (Token? comment = token.precedingComments;
comment != null;
Expand Down Expand Up @@ -312,7 +313,7 @@ class CommentSequence extends ListBase<SourceComment> {
SourceComment operator [](int index) => _comments[index];

@override
operator []=(int index, SourceComment value) =>
void operator []=(int index, SourceComment value) =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This return type is pointless. Index setters are like setters: they can never return a value anyway.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally agree, so maybe the lint needs to be updated for this case!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the lint is doing the right thing. I enabled always_declare_return_types and tested it on:

class Foo {
  get getter => 1;
  set setter(int value) => 1;
  method() => 1;
  operator [](int index) => 1;
  operator []=(int index, int value) => 1;
}

I get lint warnings on getter, method(), and operator [], but not setter or operator []=. Maybe the fix is out of sync with the lint? Or maybe this was just a mistake fixing the lint? Either way, I think you can delete this void and it should still be lint clean.

throw UnsupportedError('Comment sequence can\'t be modified.');

void _add(int linesBefore, SourceComment comment) {
Expand Down
2 changes: 1 addition & 1 deletion lib/src/front_end/delimited_list_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/token.dart';
import 'package:dart_style/src/ast_extensions.dart';
kevmoo marked this conversation as resolved.
Show resolved Hide resolved

import '../ast_extensions.dart';
import '../comment_type.dart';
import '../piece/list.dart';
import '../piece/piece.dart';
Expand Down
2 changes: 1 addition & 1 deletion lib/src/front_end/sequence_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/token.dart';
import 'package:dart_style/src/ast_extensions.dart';
import '../ast_extensions.dart';

import '../constants.dart';
import '../piece/piece.dart';
Expand Down
2 changes: 1 addition & 1 deletion lib/src/io.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Future<void> formatStdin(

var completer = Completer<void>();
var input = StringBuffer();
stdin.transform(Utf8Decoder()).listen(input.write, onDone: () {
stdin.transform(const Utf8Decoder()).listen(input.write, onDone: () {
var formatter = DartFormatter(
indent: options.indent,
pageWidth: options.pageWidth,
Expand Down
2 changes: 1 addition & 1 deletion lib/src/line_splitting/solve_state.dart
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ class SolveState {

// The way SolveStates are expanded should guarantee that we never generate
// the exact same state twice. Getting here implies that that failed.
throw 'unreachable';
throw StateError('unreachable');
}

/// Enqueues more solve states to consider based on this one.
Expand Down
Loading