Skip to content

Commit cf78037

Browse files
DanTupCommit Queue
authored andcommitted
[analysis_server] Change default leading/trailing behaviour for replaceSourceIndent
Makes behaviour of changing leading indent or trailing newlines opt-in. There are only slightly more uses of this method that don't want this (now opt-in) behaviour, although I think this is slightly less surprising (for example the trailing newline behaviour was to "add a trailing newline only if it didn't already exist" which I don't think was obvious). Mostly this was just changing the defaults and all of the call sites. However, some call sites were getting the old behaviour (inc leading/trailing whitespace), but then calling `.trim()`, so I didn't add the opt-in there because it would then be removed (they won't show up in the diff because they didn't actually get any changes). Change-Id: I2f05b3a6266aaad5ee510ce2a8bf269b26d42d5d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/329540 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Brian Wilkerson <[email protected]> Reviewed-by: Phil Quitslund <[email protected]>
1 parent 8d80d97 commit cf78037

19 files changed

+102
-68
lines changed

pkg/analysis_server/lib/src/services/completion/postfix/postfix_completion.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,8 @@ class PostfixCompletionProcessor {
350350
builder.write('try {');
351351
builder.write(eol);
352352
builder.write(utils.replaceSourceIndent(
353-
src, indent, '$indent${utils.getIndent(1)}'));
353+
src, indent, '$indent${utils.getIndent(1)}',
354+
includeLeading: true, ensureTrailingNewline: true));
354355
builder.selectHere();
355356
builder.write(indent);
356357
builder.write('}');

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@ class ConvertFlutterChildren extends ResolvedCorrectionProducer {
3030
widgetText,
3131
indentOld,
3232
indentNew,
33-
includeLeading: false,
34-
includeTrailingNewline: false,
3533
);
3634

3735
await builder.addDartFileEdit(file, (builder) {

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,8 @@ class ConvertToIfCaseStatementChain extends ResolvedCorrectionProducer {
134134
range,
135135
firstIndent,
136136
blockIndent + singleIndent,
137+
includeLeading: true,
138+
ensureTrailingNewline: true,
137139
);
138140
builder.write(code);
139141
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,8 @@ class ConvertIfStatementToSwitchStatement extends ResolvedCorrectionProducer {
158158
range,
159159
ifStatementIndent,
160160
newIndent,
161+
includeLeading: true,
162+
ensureTrailingNewline: true,
161163
);
162164

163165
builder.write(code);

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,6 @@ class FlutterConvertToChildren extends ResolvedCorrectionProducer {
7878
childArgSrc,
7979
indentOld,
8080
indentNew,
81-
includeLeading: false,
82-
includeTrailingNewline: false,
8381
);
8482
newChildArgSrc = '$prefix$newChildArgSrc,$eol$indentOld]';
8583
builder.addSimpleReplacement(range.node(childArg), newChildArgSrc);

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,6 @@ class FlutterRemoveWidget extends ResolvedCorrectionProducer {
108108
childText,
109109
indentOld,
110110
indentNew,
111-
includeLeading: false,
112-
includeTrailingNewline: false,
113111
);
114112
builder.addSimpleReplacement(range.node(widgetCreation), childText);
115113
});
@@ -128,8 +126,6 @@ class FlutterRemoveWidget extends ResolvedCorrectionProducer {
128126
childText,
129127
indentOld,
130128
indentNew,
131-
includeLeading: false,
132-
includeTrailingNewline: false,
133129
);
134130
builder.addSimpleReplacement(range.node(widgetCreation), childText);
135131
});

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ abstract class FlutterParentAndChild extends ResolvedCorrectionProducer {
4444
text,
4545
childIndent,
4646
parentIndent,
47-
includeLeading: false,
48-
includeTrailingNewline: false,
4947
);
5048
builder.write(parentIndent);
5149
builder.write(' ');
@@ -70,8 +68,6 @@ abstract class FlutterParentAndChild extends ResolvedCorrectionProducer {
7068
text,
7169
parentIndent,
7270
childIndent,
73-
includeLeading: false,
74-
includeTrailingNewline: false,
7571
);
7672
builder.write(childIndent);
7773
builder.write(' ');

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,8 +234,6 @@ abstract class _WrapMultipleWidgets extends ResolvedCorrectionProducer {
234234
src,
235235
indentOld,
236236
indentNew2,
237-
includeLeading: false,
238-
includeTrailingNewline: false,
239237
);
240238
builder.write(indentNew2);
241239
builder.write(newSrc);
@@ -314,8 +312,6 @@ abstract class _WrapSingleWidget extends ResolvedCorrectionProducer {
314312
widgetSrc,
315313
indentOld,
316314
indentNew,
317-
includeLeading: false,
318-
includeTrailingNewline: false,
319315
);
320316
widgetSrc += ',$eol$indentOld';
321317
}

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,6 @@ class FlutterWrapBuilder extends ResolvedCorrectionProducer {
4949
widgetSrc,
5050
indentOld,
5151
indentNew2,
52-
includeLeading: false,
53-
includeTrailingNewline: false,
5452
);
5553
builder.write(indentNew2);
5654
builder.write('return $widgetSrc');

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,6 @@ class FlutterWrapGeneric extends ResolvedCorrectionProducer {
4747
literalSrc,
4848
indentOld,
4949
indentList,
50-
includeLeading: false,
51-
includeTrailingNewline: false,
5250
));
5351
builder.write(',');
5452
builder.write(eol);

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,6 @@ class FlutterWrapStreamBuilder extends ResolvedCorrectionProducer {
5454
widgetSrc,
5555
indentOld,
5656
indentNew2,
57-
includeLeading: false,
58-
includeTrailingNewline: false,
5957
);
6058
builder.write(indentNew2);
6159
builder.write('return $widgetSrc');

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ class SplitAndCondition extends ResolvedCorrectionProducer {
9292
builder.addSimpleReplacement(
9393
linesRange,
9494
utils.replaceSourceRangeIndent(
95-
linesRange, thenIndentOld, thenIndentNew));
95+
linesRange, thenIndentOld, thenIndentNew,
96+
includeLeading: true, ensureTrailingNewline: true));
9697
}
9798
});
9899
}

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,9 @@ class SurroundWith extends MultiCorrectionProducer {
4848
// prepare environment
4949
var indentOld = utils.getNodePrefix(firstStatement);
5050
var indentNew = '$indentOld${utils.getIndent(1)}';
51-
var indentedCode =
52-
utils.replaceSourceRangeIndent(statementsRange, indentOld, indentNew);
51+
var indentedCode = utils.replaceSourceRangeIndent(
52+
statementsRange, indentOld, indentNew,
53+
includeLeading: true, ensureTrailingNewline: true);
5354

5455
return [
5556
_SurroundWithBlock(statementsRange, indentOld, indentNew, indentedCode),
@@ -98,8 +99,8 @@ class _SurroundWithBlock extends _SurroundWith {
9899
builder.addSimpleInsertion(statementsRange.offset, '$indentOld{$eol');
99100
builder.addSimpleReplacement(
100101
statementsRange,
101-
utils.replaceSourceRangeIndent(
102-
statementsRange, indentOld, indentNew));
102+
utils.replaceSourceRangeIndent(statementsRange, indentOld, indentNew,
103+
includeLeading: true, ensureTrailingNewline: true));
103104
builder.addSimpleInsertion(statementsRange.end, '$indentOld}$eol');
104105
});
105106
}

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

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1236,15 +1236,18 @@ class CorrectionUtils {
12361236
/// Returns the source with indentation changed from [oldIndent] to
12371237
/// [newIndent], keeping indentation of lines relative to each other.
12381238
///
1239-
/// If [includeLeading] is `false`, indentation on the first line will not be
1240-
/// altered. This should be used if the provided string is a substring of code
1241-
/// that does not begin and end at line boundaries and [oldIndent] may be
1242-
/// an empty string.
1239+
/// Indentation on the first line will only be updated if [includeLeading] is
1240+
/// `true`.
12431241
///
1244-
/// Unless [includeTrailingNewline] is `false`, a newline will be added to
1245-
/// the end of the returned code.
1242+
/// If [ensureTrailingNewline] is `true`, a newline will be added to
1243+
/// the end of the returned code if it does not already have one.
1244+
///
1245+
/// Usually [includeLeading] and [ensureTrailingNewline] will both be set
1246+
/// together when indenting a set of statements to go inside a block (as
1247+
/// opposed to just wrapping a nested expression that might span multiple
1248+
/// lines).
12461249
String replaceSourceIndent(String source, String oldIndent, String newIndent,
1247-
{bool includeLeading = true, bool includeTrailingNewline = true}) {
1250+
{bool includeLeading = false, bool ensureTrailingNewline = false}) {
12481251
// prepare STRING token ranges
12491252
var lineRanges = <SourceRange>[];
12501253
{
@@ -1262,12 +1265,17 @@ class CorrectionUtils {
12621265
var lineOffset = 0;
12631266
for (var i = 0; i < lines.length; i++) {
12641267
var line = lines[i];
1265-
var doReplaceWhitespace = i != 0 || includeLeading;
1266-
var doAppendEol = i != lines.length - 1 || includeTrailingNewline;
1267-
// last line, stop if empty
1268+
// Exit early if this is the last line and it's already empty, to avoid
1269+
// inserting any whitespace or appending an additional newline if
1270+
// [ensureTrailingNewline].
12681271
if (i == lines.length - 1 && isEmpty(line)) {
12691272
break;
12701273
}
1274+
// Don't replace whitespace on first line unless [includeLeading].
1275+
var doReplaceWhitespace = i != 0 || includeLeading;
1276+
// Don't add eol to last line unless [ensureTrailingNewline].
1277+
var doAppendEol = i != lines.length - 1 || ensureTrailingNewline;
1278+
12711279
// check if "offset" is in one of the String ranges
12721280
var inString = false;
12731281
for (var lineRange in lineRanges) {
@@ -1295,10 +1303,24 @@ class CorrectionUtils {
12951303
/// Returns the source of the given [SourceRange] with indentation changed
12961304
/// from [oldIndent] to [newIndent], keeping indentation of lines relative
12971305
/// to each other.
1306+
///
1307+
/// Indentation on the first line will only be updated if [includeLeading] is
1308+
/// `true`.
1309+
///
1310+
/// If [ensureTrailingNewline] is `true`, a newline will be added to
1311+
/// the end of the returned code if it does not already have one.
1312+
///
1313+
/// Usually [includeLeading] and [ensureTrailingNewline] will both be set
1314+
/// together when indenting a set of statements to go inside a block (as
1315+
/// opposed to just wrapping a nested expression that might span multiple
1316+
/// lines).
12981317
String replaceSourceRangeIndent(
1299-
SourceRange range, String oldIndent, String newIndent) {
1318+
SourceRange range, String oldIndent, String newIndent,
1319+
{bool includeLeading = false, bool ensureTrailingNewline = false}) {
13001320
var oldSource = getRangeText(range);
1301-
return replaceSourceIndent(oldSource, oldIndent, newIndent);
1321+
return replaceSourceIndent(oldSource, oldIndent, newIndent,
1322+
includeLeading: includeLeading,
1323+
ensureTrailingNewline: ensureTrailingNewline);
13021324
}
13031325

13041326
/// Return `true` if [selection] covers [node] and there are any

pkg/analysis_server/lib/src/services/refactoring/legacy/extract_method.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -668,7 +668,8 @@ class ExtractMethodRefactoringImpl extends RefactoringImpl
668668
if (selectionStatements != null) {
669669
var selectionIndent = utils.getNodePrefix(selectionStatements[0]);
670670
var targetIndent = '${utils.getNodePrefix(_parentMember!)} ';
671-
source = utils.replaceSourceIndent(source, selectionIndent, targetIndent);
671+
source = utils.replaceSourceIndent(source, selectionIndent, targetIndent,
672+
includeLeading: true, ensureTrailingNewline: true);
672673
}
673674
// done
674675
return source;

pkg/analysis_server/lib/src/services/refactoring/legacy/extract_widget.dart

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -524,8 +524,6 @@ class ExtractWidgetRefactoringImpl extends RefactoringImpl
524524
code,
525525
indentOld,
526526
indentNew,
527-
includeLeading: false,
528-
includeTrailingNewline: false,
529527
);
530528

531529
builder.writeln('{');
@@ -540,8 +538,7 @@ class ExtractWidgetRefactoringImpl extends RefactoringImpl
540538
var indentNew = ' ';
541539

542540
var code = utils.getRangeText(_statementsRange!);
543-
code = utils.replaceSourceIndent(code, indentOld, indentNew,
544-
includeLeading: false, includeTrailingNewline: false);
541+
code = utils.replaceSourceIndent(code, indentOld, indentNew);
545542

546543
builder.writeln('{');
547544

pkg/analysis_server/lib/src/services/refactoring/legacy/inline_method.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,8 @@ class _ReferenceProcessor {
553553
var source = _getMethodSourceForInvocation(status,
554554
ref._methodStatementsPart!, _refUtils, usage, target, arguments);
555555
source = _refUtils.replaceSourceIndent(
556-
source, ref._methodStatementsPart!._prefix, _refPrefix);
556+
source, ref._methodStatementsPart!._prefix, _refPrefix,
557+
includeLeading: true, ensureTrailingNewline: true);
557558
// do insert
558559
var edit =
559560
newSourceEdit_range(SourceRange(_refLineRange!.offset, 0), source);

pkg/analysis_server/lib/src/utilities/flutter.dart

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ class Flutter {
8282
String Function(int) getIndent,
8383
String Function(int, int) getText,
8484
String Function(String, String, String,
85-
{bool includeLeading, bool includeTrailingNewline})
85+
{bool includeLeading, bool ensureTrailingNewline})
8686
replaceSourceIndent,
8787
SourceRange Function(Expression) rangeNode) {
8888
var childLoc = namedExp.offset + 'child'.length;
@@ -113,8 +113,6 @@ class Flutter {
113113
childArgSrc,
114114
indentOld,
115115
indentNew,
116-
includeLeading: false,
117-
includeTrailingNewline: false,
118116
);
119117
newChildArgSrc = '$prefix$newChildArgSrc,$eol$indentOld]';
120118
builder.addSimpleReplacement(rangeNode(childArg), newChildArgSrc);

0 commit comments

Comments
 (0)