Improve the spacing of suggested fix stringifications.
Suggested fixes used to include line breaks, but that makes everything more confusing. Instead, trim each suggested fix, so that whitespace stays consistent.
PiperOrigin-RevId: 276478533
diff --git a/internal/tsetse/failure.ts b/internal/tsetse/failure.ts
index 317faf4..02ccd48 100644
--- a/internal/tsetse/failure.ts
+++ b/internal/tsetse/failure.ts
@@ -66,30 +66,38 @@
let fixText = '';
for (const c of f.changes) {
+ // Remove leading/trailing whitespace from the stringified suggestions:
+ // since we add line breaks after each line of stringified suggestion, and
+ // since users will manually apply the fix, there is no need to show
+ // trailing whitespace. This is however just for stringification of the
+ // fixes: the suggested fix itself still keeps trailing whitespace.
+ const printableReplacement = c.replacement.trim();
+
// Insertion.
if (c.start === c.end) {
// Try to see if that's an import.
if (c.replacement.indexOf('import') !== -1) {
- fixText += `- Add new import: ${c.replacement}\n`;
+ fixText += `- Add new import: ${printableReplacement}\n`;
} else {
// Insertion that's not a full import. This should rarely happen in
// our context, and we don't have a great message for these.
// For instance, this could be the addition of a new symbol in an
// existing import (`import {foo}` becoming `import {foo, bar}`).
fixText += `- Insert ${this.readableRange(c.start, c.end)}: ${
- c.replacement}\n`;
+ printableReplacement}\n`;
}
} else if (c.start === this.start && c.end === this.end) {
// We assume the replacement is the main part of the fix, so put that
// individual change first in `fixText`.
- fixText = `- Replace the full match with: ${c.replacement}\n` + fixText;
+ fixText = `- Replace the full match with: ${printableReplacement}\n` +
+ fixText;
} else {
// Fallback case: Use a numerical range to specify a replacement. In
// general, falling through in this case should be avoided, as it's not
// really readable without an IDE (the range can be outside of the
// matched code).
fixText = `- Replace ${this.readableRange(c.start, c.end)} with: ` +
- `${c.replacement}\n${fixText}`;
+ `${printableReplacement}\n${fixText}`;
}
}
diff --git a/internal/tsetse/tests/ban_conformance_pattern/fixer_test.ts b/internal/tsetse/tests/ban_conformance_pattern/fixer_test.ts
index 59843a6..afd9177 100644
--- a/internal/tsetse/tests/ban_conformance_pattern/fixer_test.ts
+++ b/internal/tsetse/tests/ban_conformance_pattern/fixer_test.ts
@@ -100,11 +100,20 @@
describe('adds imports', () => {
const addNamedImportFixer: Fixer = {
- getFixForFlaggedNode(n: ts.Node) {
- const ic =
+ getFixForFlaggedNode(n: ts.Node): Fix |
+ undefined {
+ const changes = [];
+ const ic1 =
maybeAddNamedImport(n.getSourceFile(), 'foo', './file_1', 'bar');
- if (ic) return {changes: [ic]};
- return;
+ if (ic1) {
+ changes.push(ic1);
+ }
+ const ic2 =
+ maybeAddNamedImport(n.getSourceFile(), 'foo2', './file_2', 'bar2');
+ if (ic2) {
+ changes.push(ic2);
+ }
+ return changes.length ? {changes} : undefined;
}
};
@@ -112,15 +121,23 @@
const results = compileAndCheck(
new ConformancePatternRule(baseConfig, addNamedImportFixer), source);
- expect(results[0]).toHaveFixMatching([{
- start: 0,
- end: 0,
- replacement: `import {foo as bar} from './file_1';\n`
- }]);
+ expect(results[0]).toHaveFixMatching([
+ {
+ start: 0,
+ end: 0,
+ replacement: `import {foo as bar} from './file_1';\n`
+ },
+ {
+ start: 0,
+ end: 0,
+ replacement: `import {foo2 as bar2} from './file_2';\n`
+ }
+ ]);
expect(results[0].fixToReadableStringInContext())
.toBe(
`Suggested fix:\n` +
- `- Add new import: import {foo as bar} from './file_1';`);
+ `- Add new import: import {foo as bar} from './file_1';\n` +
+ `- Add new import: import {foo2 as bar2} from './file_2';`);
});
it('maybeAddNamedImport already there', () => {
@@ -129,8 +146,15 @@
'import {foo as bar} from \'./file_1\';\n' + source,
'export const foo = 1;');
- expect(results[0]).toHaveNoFix();
- expect(results[0].fixToReadableStringInContext()).toBe('');
+ expect(results[0]).toHaveFixMatching([{
+ start: 37,
+ end: 37,
+ replacement: `import {foo2 as bar2} from './file_2';\n`
+ }]);
+ expect(results[0].fixToReadableStringInContext())
+ .toBe(
+ `Suggested fix:\n` +
+ `- Add new import: import {foo2 as bar2} from './file_2';`);
});
it('maybeAddNamedImport different name', () => {
@@ -140,12 +164,17 @@
'export const foo = 1;');
expect(results[0]).toHaveFixMatching([
- {start: 8, end: 8, replacement: `foo as bar, `}
+ {start: 8, end: 8, replacement: `foo as bar, `}, {
+ start: 37,
+ end: 37,
+ replacement: `import {foo2 as bar2} from './file_2';\n`
+ }
]);
expect(results[0].fixToReadableStringInContext())
.toBe(
`Suggested fix:\n` +
- `- Insert at line 1, char 9: foo as bar,`);
+ `- Insert at line 1, char 9: foo as bar,\n` +
+ `- Add new import: import {foo2 as bar2} from './file_2';`);
});
it('maybeAddNamespacedImport', () => {
diff --git a/internal/tsetse/util/testing/test_support.ts b/internal/tsetse/util/testing/test_support.ts
index e474ec7..bfbc78b 100644
--- a/internal/tsetse/util/testing/test_support.ts
+++ b/internal/tsetse/util/testing/test_support.ts
@@ -92,12 +92,8 @@
fix?: FixExpectations;
}
-type FixExpectations = [{
- fileName?: string;
- start?: number;
- end?: number;
- replacement?: string;
-}];
+type FixExpectations = Array<
+ {fileName?: string; start?: number; end?: number; replacement?: string;}>;
function failureMatchesExpectation(
