Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Commit

Permalink
[breaking] Remove createFix and just use the Replacement(s) as the …
Browse files Browse the repository at this point in the history
…fix (#2403)
  • Loading branch information
andy-hanson authored and adidahiya committed Mar 28, 2017
1 parent 41dee8f commit 4141de4
Show file tree
Hide file tree
Showing 39 changed files with 114 additions and 172 deletions.
2 changes: 1 addition & 1 deletion docs/_data/formatters.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
{
"formatterName": "json",
"description": "Formats errors as simple JSON.",
"sample": "\n[\n {\n \"endPosition\": {\n \"character\": 13,\n \"line\": 0,\n \"position\": 13\n },\n \"failure\": \"Missing semicolon\",\n \"fix\": {\n \"innerRuleName\": \"semicolon\",\n \"innerReplacements\": [\n {\n \"innerStart\": 13,\n \"innerLength\": 0,\n \"innerText\": \";\"\n }\n ]\n },\n \"name\": \"myFile.ts\",\n \"ruleName\": \"semicolon\",\n \"startPosition\": {\n \"character\": 13,\n \"line\": 0,\n \"position\": 13\n }\n }\n]",
"sample": "\n[\n {\n \"endPosition\": {\n \"character\": 13,\n \"line\": 0,\n \"position\": 13\n },\n \"failure\": \"Missing semicolon\",\n \"fix\": {\n \"innerStart\": 13,\n \"innerLength\": 0,\n \"innerText\": \";\"\n },\n \"name\": \"myFile.ts\",\n \"ruleName\": \"semicolon\",\n \"startPosition\": {\n \"character\": 13,\n \"line\": 0,\n \"position\": 13\n }\n }\n]",
"consumer": "machine"
},
{
Expand Down
11 changes: 3 additions & 8 deletions docs/formatters/json/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,9 @@
},
"failure": "Missing semicolon",
"fix": {
"innerRuleName": "semicolon",
"innerReplacements": [
{
"innerStart": 13,
"innerLength": 0,
"innerText": ";"
}
]
"innerStart": 13,
"innerLength": 0,
"innerText": ";"
},
"name": "myFile.ts",
"ruleName": "semicolon",
Expand Down
11 changes: 3 additions & 8 deletions src/formatters/jsonFormatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,9 @@ export class Formatter extends AbstractFormatter {
},
"failure": "Missing semicolon",
"fix": {
"innerRuleName": "semicolon",
"innerReplacements": [
{
"innerStart": 13,
"innerLength": 0,
"innerText": ";"
}
]
"innerStart": 13,
"innerLength": 0,
"innerText": ";"
},
"name": "myFile.ts",
"ruleName": "semicolon",
Expand Down
37 changes: 11 additions & 26 deletions src/language/rule/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import * as ts from "typescript";

import {arrayify, flatMap} from "../../utils";
import {IWalker} from "../walker";

export interface IRuleMetadata {
Expand Down Expand Up @@ -132,12 +133,20 @@ export function isTypedRule(rule: IRule): rule is ITypedRule {
}

export class Replacement {
public static applyFixes(content: string, fixes: Fix[]): string {
return this.applyAll(content, flatMap(fixes, arrayify));
}

public static applyAll(content: string, replacements: Replacement[]) {
// sort in reverse so that diffs are properly applied
replacements.sort((a, b) => b.end - a.end);
return replacements.reduce((text, r) => r.apply(text), content);
}

public static replaceNode(node: ts.Node, text: string, sourceFile?: ts.SourceFile): Replacement {
return this.replaceFromTo(node.getStart(sourceFile), node.getEnd(), text);
}

public static replaceFromTo(start: number, end: number, text: string) {
return new Replacement(start, end - start, text);
}
Expand Down Expand Up @@ -178,32 +187,6 @@ export class Replacement {
}
}

export class Fix {
public static applyAll(content: string, fixes: Fix[]) {
// accumulate replacements
let replacements: Replacement[] = [];
for (const fix of fixes) {
replacements = replacements.concat(fix.replacements);
}
return Replacement.applyAll(content, replacements);
}

constructor(private innerRuleName: string, private innerReplacements: Replacement[]) {
}

get ruleName() {
return this.innerRuleName;
}

get replacements() {
return this.innerReplacements;
}

public apply(content: string) {
return Replacement.applyAll(content, this.innerReplacements);
}
}

export class RuleFailurePosition {
constructor(private position: number, private lineAndCharacter: ts.LineAndCharacter) {
}
Expand Down Expand Up @@ -234,6 +217,8 @@ export class RuleFailurePosition {
}
}

export type Fix = Replacement | Replacement[];

export class RuleFailure {
private fileName: string;
private startPosition: RuleFailurePosition;
Expand Down
4 changes: 0 additions & 4 deletions src/language/walker/ruleWalker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,4 @@ export class RuleWalker extends SyntaxWalker implements IWalker {
public getRuleName(): string {
return this.ruleName;
}

public createFix(...replacements: Replacement[]): Fix {
return new Fix(this.ruleName, replacements);
}
}
6 changes: 1 addition & 5 deletions src/language/walker/walkContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

import * as ts from "typescript";

import { Fix, Replacement, RuleFailure } from "../rule/rule";
import { Fix, RuleFailure } from "../rule/rule";

export class WalkContext<T> {
public readonly failures: RuleFailure[] = [];
Expand All @@ -40,8 +40,4 @@ export class WalkContext<T> {
public addFailureAtNode(node: ts.Node, failure: string, fix?: Fix) {
this.addFailure(node.getStart(this.sourceFile), node.getEnd(), failure, fix);
}

public createFix(...replacements: Replacement[]) {
return new Fix(this.ruleName, replacements);
}
}
4 changes: 2 additions & 2 deletions src/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import { isError, showWarningOnce } from "./error";
import { findFormatter } from "./formatterLoader";
import { ILinterOptions, LintResult } from "./index";
import { IFormatter } from "./language/formatter/formatter";
import { Fix, IRule, isTypedRule, RuleFailure, RuleSeverity } from "./language/rule/rule";
import { Fix, IRule, isTypedRule, Replacement, RuleFailure, RuleSeverity } from "./language/rule/rule";
import * as utils from "./language/utils";
import { loadRules } from "./ruleLoader";
import { arrayify, dedent } from "./utils";
Expand Down Expand Up @@ -108,7 +108,7 @@ class Linter {
source = fs.readFileSync(fileName, { encoding: "utf-8" });
if (fixes.length > 0) {
this.fixes = this.fixes.concat(ruleFailures);
source = Fix.applyAll(source, fixes);
source = Replacement.applyFixes(source, fixes);
fs.writeFileSync(fileName, source, { encoding: "utf-8" });

// reload AST if file is modified
Expand Down
4 changes: 2 additions & 2 deletions src/rules/alignRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,10 @@ class AlignWalker extends Lint.AbstractWalker<Options> {
const diff = alignToColumn - pos.character;
let fix: Lint.Fix | undefined;
if (0 < diff) {
fix = this.createFix(Lint.Replacement.appendText(start, " ".repeat(diff)));
fix = Lint.Replacement.appendText(start, " ".repeat(diff));
} else if (node.pos <= start + diff && /^\s+$/.test(sourceFile.text.substring(start + diff, start))) {
// only delete text if there is only whitespace
fix = this.createFix(Lint.Replacement.deleteText(start + diff, -diff));
fix = Lint.Replacement.deleteText(start + diff, -diff);
}
this.addFailure(start, node.end, kind + Rule.FAILURE_STRING_SUFFIX, fix);
}
Expand Down
10 changes: 5 additions & 5 deletions src/rules/arrayTypeRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ class ArrayTypeWalker extends Lint.RuleWalker {
// Add a space if the type is preceded by 'as' and the node has no leading whitespace
const space = !parens && node.parent!.kind === ts.SyntaxKind.AsExpression &&
node.getStart() === node.getFullStart() ? " " : "";
const fix = this.createFix(
const fix = [
this.createReplacement(typeName.getStart(), parens, space + "Array<"),
// Delete the square brackets and replace with an angle bracket
this.createReplacement(typeName.getEnd() - parens, node.getEnd() - typeName.getEnd() + parens, ">"),
);
];
this.addFailureAtNode(node, failureString, fix);
}

Expand All @@ -83,20 +83,20 @@ class ArrayTypeWalker extends Lint.RuleWalker {
const typeArgs = node.typeArguments;
if (!typeArgs || typeArgs.length === 0) {
// Create an 'any' array
const fix = this.createFix(this.createReplacement(node.getStart(), node.getWidth(), "any[]"));
const fix = this.createReplacement(node.getStart(), node.getWidth(), "any[]");
this.addFailureAtNode(node, failureString, fix);
} else if (typeArgs && typeArgs.length === 1 && (!this.hasOption(OPTION_ARRAY_SIMPLE) || this.isSimpleType(typeArgs[0]))) {
const type = typeArgs[0];
const typeStart = type.getStart();
const typeEnd = type.getEnd();
const parens = type.kind === ts.SyntaxKind.UnionType ||
type.kind === ts.SyntaxKind.FunctionType || type.kind === ts.SyntaxKind.IntersectionType;
const fix = this.createFix(
const fix = [
// Delete Array and the first angle bracket
this.createReplacement(node.getStart(), typeStart - node.getStart(), parens ? "(" : ""),
// Delete the last angle bracket and replace with square brackets
this.createReplacement(typeEnd, node.getEnd() - typeEnd, (parens ? ")" : "") + "[]"),
);
];
this.addFailureAtNode(node, failureString, fix);
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/rules/arrowParensRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,17 @@ function walk(ctx: Lint.WalkContext<Options>) {
const parameter = node.parameters[0];
const start = parameter.getStart(ctx.sourceFile);
const end = parameter.end;
ctx.addFailure(start, end, Rule.FAILURE_STRING_MISSING, ctx.createFix(
ctx.addFailure(start, end, Rule.FAILURE_STRING_MISSING, [
Lint.Replacement.appendText(start, "("),
Lint.Replacement.appendText(end, ")"),
));
]);
}
} else if (ctx.options.banSingleArgParens) {
const closeParen = getChildOfKind(node, ts.SyntaxKind.CloseParenToken)!;
ctx.addFailureAtNode(node.parameters[0], Rule.FAILURE_STRING_EXISTS, ctx.createFix(
ctx.addFailureAtNode(node.parameters[0], Rule.FAILURE_STRING_EXISTS, [
Lint.Replacement.deleteText(openParen.end - 1, 1),
Lint.Replacement.deleteText(closeParen.end - 1, 1),
));
]);
}
}
return ts.forEachChild(node, cb);
Expand Down
4 changes: 2 additions & 2 deletions src/rules/arrowReturnShorthandRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class Walker extends Lint.RuleWalker {

const anyComments = hasComments(arrow) || hasComments(openBrace) || hasComments(statement) || hasComments(returnKeyword) ||
hasComments(expr) || (semicolon && hasComments(semicolon)) || hasComments(closeBrace);
return anyComments ? undefined : this.createFix(
return anyComments ? undefined : [
// Object literal must be wrapped in `()`
...(expr.kind === ts.SyntaxKind.ObjectLiteralExpression ? [
this.appendText(expr.getStart(), "("),
Expand All @@ -90,7 +90,7 @@ class Walker extends Lint.RuleWalker {
this.deleteFromTo(statement.getStart(), expr.getStart()),
// " }" (may include semicolon)
this.deleteFromTo(expr.end, closeBrace.end),
);
];

function hasComments(node: ts.Node): boolean {
return ts.getTrailingCommentRanges(text, node.getEnd()) !== undefined;
Expand Down
4 changes: 1 addition & 3 deletions src/rules/eoflineRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@ export class Rule extends Lint.Rules.AbstractRule {
let fix: Lint.Fix | undefined;
const lines = sourceFile.getLineStarts();
if (lines.length > 1) {
fix = new Lint.Fix(this.ruleName, [
Lint.Replacement.appendText(length, sourceFile.text[lines[1] - 2] === "\r" ? "\r\n" : "\n"),
]);
fix = Lint.Replacement.appendText(length, sourceFile.text[lines[1] - 2] === "\r" ? "\r\n" : "\n");
}

return this.filterFailures([
Expand Down
8 changes: 2 additions & 6 deletions src/rules/linebreakStyleRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,10 @@ function walk(ctx: Lint.WalkContext<boolean>) {
const lineEnd = lineStarts[i] - 1;
if (sourceText[lineEnd - 1] === "\r") {
if (!expectedCr) {
ctx.addFailure(lineStarts[i - 1], lineEnd - 1, Rule.FAILURE_LF, ctx.createFix(
Lint.Replacement.deleteText(lineEnd - 1, 1),
));
ctx.addFailure(lineStarts[i - 1], lineEnd - 1, Rule.FAILURE_LF, Lint.Replacement.deleteText(lineEnd - 1, 1));
}
} else if (expectedCr) {
ctx.addFailure(lineStarts[i - 1], lineEnd, Rule.FAILURE_CRLF, ctx.createFix(
Lint.Replacement.appendText(lineEnd, "\r"),
));
ctx.addFailure(lineStarts[i - 1], lineEnd, Rule.FAILURE_CRLF, Lint.Replacement.appendText(lineEnd, "\r"));
}
}
}
20 changes: 1 addition & 19 deletions src/rules/memberOrderingRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import * as ts from "typescript";

import * as Lint from "../index";
import {flatMap, mapDefined} from "../utils";

const OPTION_ORDER = "order";
const OPTION_ALPHABETIZE = "alphabetize";
Expand Down Expand Up @@ -478,22 +479,3 @@ function nameString(name: ts.PropertyName): string {
return "";
}
}

function mapDefined<T, U>(inputs: T[], getOutput: (input: T) => U | undefined): U[] {
const out = [];
for (const input of inputs) {
const output = getOutput(input);
if (output !== undefined) {
out.push(output);
}
}
return out;
}

function flatMap<T, U>(inputs: T[], getOutputs: (input: T) => U[]): U[] {
const out = [];
for (const input of inputs) {
out.push(...getOutputs(input));
}
return out;
}
4 changes: 2 additions & 2 deletions src/rules/noAngleBracketTypeAssertionRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ function walk(ctx: Lint.WalkContext<void>) {
return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void {
if (isTypeAssertion(node)) {
const start = node.getStart(ctx.sourceFile);
ctx.addFailure(start, node.end, Rule.FAILURE_STRING, ctx.createFix(
ctx.addFailure(start, node.end, Rule.FAILURE_STRING, [
Lint.Replacement.appendText(node.end, ` as ${ node.type.getText(ctx.sourceFile) }`),
Lint.Replacement.deleteFromTo(start, node.expression.getStart(ctx.sourceFile)),
));
]);
}
return ts.forEachChild(node, cb);
});
Expand Down
4 changes: 1 addition & 3 deletions src/rules/noAnyRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ function walk(ctx: Lint.WalkContext<void>) {
return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void {
if (node.kind === ts.SyntaxKind.AnyKeyword) {
const start = node.end - 3;
return ctx.addFailure(start, node.end, Rule.FAILURE_STRING, ctx.createFix(
new Lint.Replacement(start, 3, "{}"),
));
return ctx.addFailure(start, node.end, Rule.FAILURE_STRING, new Lint.Replacement(start, 3, "{}"));
}
return ts.forEachChild(node, cb);
});
Expand Down
2 changes: 1 addition & 1 deletion src/rules/noBooleanLiteralCompareRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class Walker extends Lint.ProgramAwareRuleWalker {
}
}

this.addFailureAtNode(expression, Rule.FAILURE_STRING(negate), this.createFix(...replacements));
this.addFailureAtNode(expression, Rule.FAILURE_STRING(negate), replacements);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/rules/noConsecutiveBlankLinesRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,14 @@ function walk(ctx: Lint.WalkContext<number>) {
const templateRanges = getTemplateRanges(ctx.sourceFile);
for (const possibleFailure of possibleFailures) {
if (!templateRanges.some((template) => template.pos < possibleFailure.pos && possibleFailure.pos < template.end)) {
ctx.addFailureAt(possibleFailure.pos, 1, failureString, ctx.createFix(
ctx.addFailureAt(possibleFailure.pos, 1, failureString, [
Lint.Replacement.deleteFromTo(
// special handling for fixing blank lines at the end of the file
// to fix this we need to cut off the line break of the last allowed blank line, too
possibleFailure.end === sourceText.length ? getStartOfLineBreak(sourceText, possibleFailure.pos) : possibleFailure.pos,
possibleFailure.end,
),
));
]);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/rules/noInferrableTypesRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ class NoInferrableTypesWalker extends Lint.AbstractWalker<IOptions> {
if (failure != null) {
this.addFailureAtNode(node.type,
Rule.FAILURE_STRING_FACTORY(failure),
this.createFix(Lint.Replacement.deleteFromTo(node.name.end, node.type.end)),
Lint.Replacement.deleteFromTo(node.name.end, node.type.end),
);
}
}
Expand Down
7 changes: 4 additions & 3 deletions src/rules/noStringThrowRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,10 @@ class Walker extends Lint.RuleWalker {
public visitThrowStatement(node: ts.ThrowStatement) {
const {expression} = node;
if (this.stringConcatRecursive(expression)) {
const fix = this.createFix(this.createReplacement(expression.getStart(),
expression.getEnd() - expression.getStart(),
`new Error(${expression.getText()})`));
const fix = this.createReplacement(
expression.getStart(),
expression.getEnd() - expression.getStart(),
`new Error(${expression.getText()})`);
this.addFailure(this.createFailure(
node.getStart(), node.getWidth(), Rule.FAILURE_STRING, fix));
}
Expand Down
4 changes: 2 additions & 2 deletions src/rules/noTrailingWhitespaceRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,9 @@ function walk(ctx: Lint.WalkContext<Options>) {
}
for (const possibleFailure of possibleFailures) {
if (!excludedRanges.some((range) => range.pos < possibleFailure.pos && possibleFailure.pos < range.end)) {
ctx.addFailure(possibleFailure.pos, possibleFailure.end, Rule.FAILURE_STRING, ctx.createFix(
ctx.addFailure(possibleFailure.pos, possibleFailure.end, Rule.FAILURE_STRING,
Lint.Replacement.deleteFromTo(possibleFailure.pos, possibleFailure.end),
));
);
}
}
}
Expand Down
Loading

0 comments on commit 4141de4

Please sign in to comment.