Apply review feedback.
diff --git a/lib/src/argument_list_visitor.dart b/lib/src/argument_list_visitor.dart
index aa6d28a..a22864f 100644
--- a/lib/src/argument_list_visitor.dart
+++ b/lib/src/argument_list_visitor.dart
@@ -172,7 +172,10 @@
this._allArguments,
this._arguments,
this._functions,
- this._argumentsAfterFunctions);
+ this._argumentsAfterFunctions) {
+ assert(_functions == null || _argumentsAfterFunctions != null,
+ 'If _functions is passed, _argumentsAfterFunctions must be too.');
+ }
/// Builds chunks for the argument list.
void visit() {
@@ -187,20 +190,21 @@
_visitor.builder.endSpan();
- if (_functions != null) {
+ var functions = _functions;
+ if (functions != null) {
// TODO(rnystrom): It might look better to treat the parameter list of the
// first function as if it were an argument in the preceding argument list
// instead of just having this little solo split here. That would try to
// keep the parameter list with other arguments when possible, and, I
// think, generally look nicer.
- if (_functions!.first == _allArguments.first) {
+ if (functions.first == _allArguments.first) {
_visitor.soloZeroSplit();
} else {
_visitor.soloSplit();
}
- for (var argument in _functions!) {
- if (argument != _functions!.first) _visitor.space();
+ for (var argument in functions) {
+ if (argument != functions.first) _visitor.space();
_visitor.visit(argument);
@@ -437,11 +441,12 @@
void _visitArgument(
SourceVisitor visitor, ArgumentRule rule, Expression argument) {
// If we're about to write a block argument, handle it specially.
- if (_blocks.containsKey(argument)) {
+ var argumentBlock = _blocks[argument];
+ if (argumentBlock != null) {
rule.disableSplitOnInnerRules();
// Tell it to use the rule we've already created.
- visitor.beforeBlock(_blocks[argument]!, blockRule!, previousSplit);
+ visitor.beforeBlock(argumentBlock, blockRule!, previousSplit);
} else if (_allArguments.length > 1) {
// Edge case: Only bump the nesting if there are multiple arguments. This
// lets us avoid spurious indentation in cases like:
@@ -465,7 +470,7 @@
visitor.visit(argument);
}
- if (_blocks.containsKey(argument)) {
+ if (argumentBlock != null) {
rule.enableSplitOnInnerRules();
} else if (_allArguments.length > 1) {
visitor.builder.endBlockArgumentNesting();
diff --git a/lib/src/call_chain_visitor.dart b/lib/src/call_chain_visitor.dart
index df6bd9b..5af43f7 100644
--- a/lib/src/call_chain_visitor.dart
+++ b/lib/src/call_chain_visitor.dart
@@ -242,12 +242,13 @@
// If there are block calls, end the chain and write those without any
// extra indentation.
- if (_blockCalls != null) {
+ var blockCalls = _blockCalls;
+ if (blockCalls != null) {
_enableRule();
_visitor.zeroSplit();
_disableRule();
- for (var blockCall in _blockCalls!) {
+ for (var blockCall in blockCalls) {
blockCall.write(this);
}
diff --git a/lib/src/chunk.dart b/lib/src/chunk.dart
index aad9eb1..02dbfb6 100644
--- a/lib/src/chunk.dart
+++ b/lib/src/chunk.dart
@@ -87,8 +87,8 @@
/// someFunctionName(argument, argument,
/// argument, anotherFunction(argument,
/// argument));
- NestingLevel? get nesting => _nesting;
- NestingLevel? _nesting;
+ NestingLevel get nesting => _nesting;
+ late NestingLevel _nesting;
/// If this chunk marks the beginning of a block, this contains the child
/// chunks and other data about that nested block.
@@ -251,14 +251,15 @@
if (_isDouble == true) parts.add('double');
if (_flushLeft == true) parts.add('flush');
- if (_rule == null) {
+ var rule = _rule;
+ if (rule == null) {
parts.add('(no split)');
} else {
- parts.add(rule!.toString());
- if (rule!.isHardened) parts.add('(hard)');
+ parts.add(rule.toString());
+ if (rule.isHardened) parts.add('(hard)');
- if (rule!.constrainedRules.isNotEmpty) {
- parts.add("-> ${rule!.constrainedRules.join(' ')}");
+ if (rule.constrainedRules.isNotEmpty) {
+ parts.add("-> ${rule.constrainedRules.join(' ')}");
}
}
diff --git a/lib/src/chunk_builder.dart b/lib/src/chunk_builder.dart
index e8b5505..a17c89a 100644
--- a/lib/src/chunk_builder.dart
+++ b/lib/src/chunk_builder.dart
@@ -884,7 +884,7 @@
var chunk = _chunks[i];
if (!chunk.rule!.isHardened) return false;
- if (chunk.nesting!.isNested) return false;
+ if (chunk.nesting.isNested) return false;
if (chunk.isBlock) return false;
return true;
diff --git a/lib/src/debug.dart b/lib/src/debug.dart
index f796af3..5934afd 100644
--- a/lib/src/debug.dart
+++ b/lib/src/debug.dart
@@ -146,7 +146,7 @@
writeIf(chunk.indent != null && chunk.indent != 0,
() => 'indent ${chunk.indent}');
- writeIf(chunk.nesting?.indent != 0, () => 'nest ${chunk.nesting}');
+ writeIf(chunk.nesting.indent != 0, () => 'nest ${chunk.nesting}');
writeIf(chunk.flushLeft, () => 'flush');
diff --git a/lib/src/io.dart b/lib/src/io.dart
index 960236e..c0064b4 100644
--- a/lib/src/io.dart
+++ b/lib/src/io.dart
@@ -14,7 +14,7 @@
import 'exceptions.dart';
import 'source_code.dart';
-/// Reads input from stdin until it's closed, and the formats it.
+/// Reads and formats input from stdin until closed.
void formatStdin(FormatterOptions options, List<int>? selection, String name) {
var selectionStart = 0;
var selectionLength = 0;
diff --git a/lib/src/line_splitting/line_splitter.dart b/lib/src/line_splitting/line_splitter.dart
index 3c0d90d..8ef5926 100644
--- a/lib/src/line_splitting/line_splitter.dart
+++ b/lib/src/line_splitting/line_splitter.dart
@@ -118,9 +118,6 @@
/// and can stop looking.
final _queue = SolveStateQueue();
- /// The lowest cost solution found so far.
- SolveState? _bestSolution;
-
/// Creates a new splitter for [_writer] that tries to fit [chunks] into the
/// page width.
LineSplitter(this.writer, List<Chunk> chunks, int blockIndentation,
@@ -162,20 +159,22 @@
// Start with a completely unbound, unsplit solution.
_queue.add(SolveState(this, RuleSet(rules.length)));
+ SolveState? bestSolution;
+
var attempts = 0;
while (_queue.isNotEmpty) {
var state = _queue.removeFirst();
- if (state.isBetterThan(_bestSolution)) {
- _bestSolution = state;
+ if (state.isBetterThan(bestSolution)) {
+ bestSolution = state;
// Since we sort solutions by cost the first solution we find that
// fits is the winner.
- if (_bestSolution!.overflowChars == 0) break;
+ if (bestSolution.overflowChars == 0) break;
}
if (debug.traceSplitter) {
- var best = state == _bestSolution ? ' (best)' : '';
+ var best = state == bestSolution ? ' (best)' : '';
debug.log('$state$best');
debug.dumpLines(chunks, firstLineIndent, state.splits);
debug.log();
@@ -188,12 +187,12 @@
}
if (debug.traceSplitter) {
- debug.log('$_bestSolution (winner)');
- debug.dumpLines(chunks, firstLineIndent, _bestSolution!.splits);
+ debug.log('$bestSolution (winner)');
+ debug.dumpLines(chunks, firstLineIndent, bestSolution!.splits);
debug.log();
}
- return _bestSolution!.splits;
+ return bestSolution!.splits;
}
void enqueue(SolveState state) {
diff --git a/lib/src/line_splitting/solve_state.dart b/lib/src/line_splitting/solve_state.dart
index 47ec632..708353e 100644
--- a/lib/src/line_splitting/solve_state.dart
+++ b/lib/src/line_splitting/solve_state.dart
@@ -83,7 +83,7 @@
/// The constraints the bound rules in this state have on the remaining
/// unbound rules.
- Map<Rule, int>? _constraints;
+ late final Map<Rule, int> _constraints = _initConstraints();
/// The unbound rule values that are disallowed because they would place
/// invalid constraints on the currently bound values.
@@ -96,14 +96,16 @@
///
/// It's important to track this, because we can't allow to states to overlap
/// if one permits more values for some unbound rule than the other does.
- Map<Rule, Set<int>>? _unboundConstraints;
+ late final Map<Rule, Set<int>> _unboundConstraints =
+ _initUnboundConstraints();
/// The bound rules that appear inside lines also containing unbound rules.
///
/// By appearing in the same line, it means these bound rules may affect the
/// results of binding those unbound rules. This is used to tell if two
/// states may diverge by binding unbound rules or not.
- Set<Rule>? _boundRulesInUnboundLines;
+ late final Set<Rule> _boundRulesInUnboundLines =
+ _initBoundRulesInUnboundLines();
SolveState(this._splitter, this._ruleValues) {
_calculateSplits();
@@ -233,38 +235,31 @@
bool _isOverlapping(SolveState other) {
// Lines that contain both bound and unbound rules must have the same
// bound values.
- var boundRulesInUnboundLines = _ensureBoundRulesInUnboundLines();
- var otherBoundRulesInUnboundLines = other._ensureBoundRulesInUnboundLines();
-
- if (boundRulesInUnboundLines.length !=
- otherBoundRulesInUnboundLines.length) {
+ if (_boundRulesInUnboundLines.length !=
+ other._boundRulesInUnboundLines.length) {
return false;
}
- for (var rule in boundRulesInUnboundLines) {
- if (!otherBoundRulesInUnboundLines.contains(rule)) return false;
+ for (var rule in _boundRulesInUnboundLines) {
+ if (!other._boundRulesInUnboundLines.contains(rule)) return false;
if (_ruleValues.getValue(rule) != other._ruleValues.getValue(rule)) {
return false;
}
}
- var constraints = _ensureConstraints();
- var otherConstraints = other._ensureConstraints();
- if (constraints.length != otherConstraints.length) return false;
+ if (_constraints.length != other._constraints.length) return false;
- for (var rule in constraints.keys) {
- if (constraints[rule] != otherConstraints[rule]) return false;
+ for (var rule in _constraints.keys) {
+ if (_constraints[rule] != other._constraints[rule]) return false;
}
- var unboundConstraints = _ensureUnboundConstraints();
- var otherUnboundConstraints = other._ensureUnboundConstraints();
- if (unboundConstraints.length != otherUnboundConstraints.length) {
+ if (_unboundConstraints.length != other._unboundConstraints.length) {
return false;
}
- for (var rule in unboundConstraints.keys) {
- var disallowed = unboundConstraints[rule]!;
- var otherDisallowed = otherUnboundConstraints[rule]!;
+ for (var rule in _unboundConstraints.keys) {
+ var disallowed = _unboundConstraints[rule]!;
+ var otherDisallowed = other._unboundConstraints[rule]!;
if (disallowed.length != otherDisallowed.length) return false;
for (var value in disallowed) {
@@ -284,8 +279,8 @@
for (var i = 0; i < _splitter.chunks.length - 1; i++) {
var chunk = _splitter.chunks[i];
if (chunk.rule!.isSplit(getValue(chunk.rule!), chunk)) {
- usedNestingLevels.add(chunk.nesting!);
- chunk.nesting!.clearTotalUsedIndent();
+ usedNestingLevels.add(chunk.nesting);
+ chunk.nesting.clearTotalUsedIndent();
}
}
@@ -303,7 +298,7 @@
indent = _splitter.blockIndentation + chunk.indent!;
// And any expression nesting.
- indent += chunk.nesting!.totalUsedIndent;
+ indent += chunk.nesting.totalUsedIndent;
if (chunk.indentBlock(getValue)) indent += Indent.expression;
}
@@ -391,8 +386,8 @@
// by construction. Instead, this outlaws it by penalizing it very
// heavily if it happens to get this far.
if (previousNesting != null &&
- chunk.nesting!.totalUsedIndent != 0 &&
- chunk.nesting!.totalUsedIndent == previousNesting.totalUsedIndent &&
+ chunk.nesting.totalUsedIndent != 0 &&
+ chunk.nesting.totalUsedIndent == previousNesting.totalUsedIndent &&
!identical(chunk.nesting, previousNesting)) {
_overflowChars += 10000;
}
@@ -443,16 +438,13 @@
return added;
}
- /// Lazily initializes the [_boundInUnboundLines], which is needed to compare
+ /// Used to lazy initialize [_boundInUnboundLines], which is needed to compare
/// two states for overlap.
///
/// We do this lazily because the calculation is a bit slow, and is only
/// needed when we have two states with the same score.
- Set<Rule> _ensureBoundRulesInUnboundLines() {
- var rules = _boundRulesInUnboundLines;
- if (rules != null) return rules;
-
- rules = <Rule>{};
+ Set<Rule> _initBoundRulesInUnboundLines() {
+ var rules = <Rule>{};
var boundInLine = <Rule>{};
var hasUnbound = false;
@@ -475,19 +467,15 @@
}
if (hasUnbound) rules.addAll(boundInLine);
- _boundRulesInUnboundLines = rules;
return rules;
}
- /// Lazily initializes the [_constraints], which is needed to compare two
- /// states for overlap.
+ /// Used to lazy initializes the [_constraints], which is needed to compare
+ /// two states for overlap.
///
/// We do this lazily because the calculation is a bit slow, and is only
/// needed when we have two states with the same score.
- Map<Rule, int> _ensureConstraints() {
- var constraints = _constraints;
- if (constraints != null) return constraints;
-
+ Map<Rule, int> _initConstraints() {
_unboundRules = <Rule>{};
_boundRules = <Rule>{};
@@ -499,7 +487,7 @@
}
}
- constraints = <Rule, int>{};
+ var constraints = <Rule, int>{};
for (var bound in _boundRules) {
for (var unbound in bound.constrainedRules) {
@@ -513,24 +501,19 @@
}
}
- _constraints = constraints;
return constraints;
}
- /// Lazily initializes the [_unboundConstraints], which is needed to compare
- /// two states for overlap.
+ /// Used to lazy initialize the [_unboundConstraints], which is needed to
+ /// compare two states for overlap.
///
/// We do this lazily because the calculation is a bit slow, and is only
/// needed when we have two states with the same score.
- ///
- /// [_ensureConstraints()] should be called first.
- Map<Rule, Set<int>> _ensureUnboundConstraints() {
- var unboundConstraints = _unboundConstraints;
- if (unboundConstraints != null) return unboundConstraints;
-
- unboundConstraints = <Rule, Set<int>>{};
+ Map<Rule, Set<int>> _initUnboundConstraints() {
+ var unboundConstraints = <Rule, Set<int>>{};
for (var unbound in _unboundRules) {
- Set<int>? disallowedValues;
+ // Lazily create and add the set to the constraints only if needed.
+ late final disallowedValues = unboundConstraints[unbound] = {};
for (var bound in unbound.constrainedRules) {
if (!_boundRules.contains(bound)) continue;
@@ -553,17 +536,11 @@
continue;
}
- if (disallowedValues == null) {
- disallowedValues = <int>{};
- unboundConstraints[unbound] = disallowedValues;
- }
-
disallowedValues.add(value);
}
}
}
- _unboundConstraints = unboundConstraints;
return unboundConstraints;
}
