Skip to content

Commit

Permalink
Create a fixer for classes-constants lint rule (#2969)
Browse files Browse the repository at this point in the history
* Create a fixer for classes-constants lint rule

1. The lint rule will now fail the entire node, instead of just the pt-* string
2. The lint rule now has a fixer that will replace the pt-* string with the blueprint import

* PR comments

* Add DOM library to make compile work without skipLibCheck

* Undo regression to tslint error reporting
  • Loading branch information
alexthemark authored and giladgray committed Oct 4, 2018
1 parent 839c14b commit 35327b7
Show file tree
Hide file tree
Showing 7 changed files with 199 additions and 13 deletions.
133 changes: 122 additions & 11 deletions packages/tslint-config/src/rules/blueprintClassesConstantsRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
*/

import * as Lint from "tslint";
import * as utils from "tsutils";
import * as ts from "typescript";

// detect "pt-" *prefix*: not preceded by letter or dash
const PATTERN = /[^\w-<]pt-[\w-]+/;
import { addImportToFile } from "./utils/addImportToFile";

export class Rule extends Lint.Rules.AbstractRule {
public static metadata: Lint.IRuleMetadata = {
Expand All @@ -29,15 +28,127 @@ export class Rule extends Lint.Rules.AbstractRule {
}
}

function walk(ctx: Lint.WalkContext<void>): void {
return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void {
interface IMatchingString {
match: string;
index: number;
}

function walk(ctx: Lint.WalkContext<void>) {
let shouldFixImports = true;
return ts.forEachChild(ctx.sourceFile, callback);

function callback(node: ts.Node): void {
if (ts.isStringLiteralLike(node) || ts.isTemplateExpression(node)) {
const match = PATTERN.exec(node.getFullText());
if (match != null) {
// ignore first character of match: negated character class
ctx.addFailureAt(node.getFullStart() + match.index + 1, match[0].length - 1, Rule.FAILURE_STRING);
const ptMatches: IMatchingString[] = [];
const fullText = node.getFullText();
let currentMatch: RegExpExecArray | null;
// tslint:disable-next-line:no-conditional-assignment
while ((currentMatch = BLUEPRINT_CLASSNAME_PATTERN.exec(fullText)) != null) {
const fullBlueprintName = currentMatch[1]; // e.g. pt-breadcrumb
const blueprintClassName = getBlueprintClassName(fullBlueprintName); // e.g. breadcrumb

// See if we should ignore this class or not.
if (blueprintClassName == null || shouldIgnoreBlueprintClass(blueprintClassName)) {
continue;
} else {
ptMatches.push({ match: fullBlueprintName, index: currentMatch.index });
}
}
if (ptMatches.length > 0) {
ctx.addFailureAt(
node.getFullStart() + ptMatches[0].index + 1,
ptMatches[0].match.length,
Rule.FAILURE_STRING,
getReplacement(node, ptMatches.map(p => p.match), ctx.sourceFile, shouldFixImports),
);
shouldFixImports = false;
}
}
return ts.forEachChild(node, cb);
});

return ts.forEachChild(node, callback);
}
}

function getReplacement(
node: ts.StringLiteralLike | ts.TemplateExpression,
ptClassStrings: string[],
file: ts.SourceFile,
shouldFixImport: boolean,
) {
const replacements: Lint.Replacement[] = [];

// We may need to add a blueprint import to the top of the file. We only want to do this once per file, otherwise,
// we'll keep stacking imports and mess things up.
if (shouldFixImport) {
replacements.push(addImportToFile(file, ["Classes"], "@blueprintjs/core"));
}

if (utils.isStringLiteral(node)) {
// remove all illegal classnames, then slice off the quotes, and trim any remaining white space
const stringWithoutPtClasses = ptClassStrings
.reduce((value, cssClass) => {
return value.replace(cssClass, "");
}, node.getText())
.slice(1, -1)
.trim();
const templateStrings = ptClassStrings.map(n => `\${${convertPtClassName(n)}}`).join(" ");
if (stringWithoutPtClasses.length > 0) {
const replacement = `\`${templateStrings} ${stringWithoutPtClasses}\``;
replacements.push(
new Lint.Replacement(node.getStart(), node.getWidth(), wrapForParent(replacement, node, node.parent)),
);
} else {
const replacement =
ptClassStrings.length === 1 ? convertPtClassName(ptClassStrings[0]) : `\`${templateStrings}\``;
replacements.push(
new Lint.Replacement(node.getStart(), node.getWidth(), wrapForParent(replacement, node, node.parent)),
);
}
} else if (utils.isTemplateExpression(node) || utils.isNoSubstitutionTemplateLiteral(node)) {
let replacementText = node.getText();
ptClassStrings.forEach(classString => {
const classReplacement = `\${${convertPtClassName(classString)}}`;
replacementText = replacementText.replace(classString, classReplacement);
});
replacements.push(new Lint.Replacement(node.getStart(), node.getWidth(), replacementText));
}
return replacements;
}

function wrapForParent(statement: string, node: ts.Node, parentNode: ts.Node | undefined) {
if (parentNode === undefined) {
return statement;
} else if (utils.isJsxAttribute(parentNode)) {
return `{${statement}}`;
} else if (utils.isExpressionStatement(parentNode)) {
return `[${statement}]`;
// If we're changing the key, it will be child index 0 and we need to wrap it.
// Else, we're changing a value, and there's no need to wrap
} else if (utils.isPropertyAssignment(parentNode) && parentNode.getChildAt(0) === node) {
return `[${statement}]`;
} else {
return statement;
}
}

function convertPtClassName(text: string) {
const className = text
.replace("pt-", "")
.replace(/-/g, "_")
.toUpperCase();
return `Classes.${className}`;
}

const BLUEPRINT_CLASSNAME_PATTERN = /[^\w-<.](pt-[\w-]+)/g;

function getBlueprintClassName(fullClassName: string): string | undefined {
if (fullClassName.length < 3) {
return undefined;
} else {
return fullClassName.slice(3);
}
}

function shouldIgnoreBlueprintClass(blueprintClassName: string): boolean {
return blueprintClassName.startsWith("icon");
}
57 changes: 57 additions & 0 deletions packages/tslint-config/src/rules/utils/addImportToFile.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright 2018 Palantir Technologies, Inc. All rights reserved.
*
* Licensed under the terms of the LICENSE file distributed with this project.
*/

import { Replacement } from "tslint";
import * as utils from "tsutils";
import * as ts from "typescript";

export function addImportToFile(file: ts.SourceFile, imports: string[], packageName: string) {
const packageToModify = file.statements.find(
statement => utils.isImportDeclaration(statement) && statement.moduleSpecifier.getText() === `"${packageName}"`,
) as ts.ImportDeclaration;
if (
packageToModify &&
packageToModify.importClause &&
packageToModify.importClause.namedBindings &&
utils.isNamedImports(packageToModify.importClause.namedBindings)
) {
const existingImports = packageToModify.importClause.namedBindings.elements.map(el => el.name.getText());
// Poor man's lodash.uniq without the dep.
const newImports = Array.from(new Set(existingImports.concat(imports))).sort((a, b) =>
a.toLowerCase().localeCompare(b.toLowerCase()),
);
const importString = `{ ${newImports.join(", ")} }`;
return Replacement.replaceNode(packageToModify.importClause.namedBindings, importString);
} else {
// we always place the import in alphabetical order. If imports are already alpha-ordered, this will act nicely
// with existing lint rules. If imports are not alpha-ordered, this may appear weird.
const allImports = file.statements.filter(utils.isImportDeclaration);
const newImportIndex = allImports.findIndex(imp => {
// slice the quotes off each module specifier
return compare(imp.moduleSpecifier.getText().slice(1, -1), packageName) === 1;
});
const startIndex = newImportIndex === -1 ? 0 : allImports[newImportIndex].getStart();
return Replacement.appendText(startIndex, `import { ${imports.join(", ")} } from "${packageName}";\n`);
}
}

function isLow(value: string) {
return value[0] === "." || value[0] === "/";
}

// taken from tslint orderedImportRules
function compare(a: string, b: string): 0 | 1 | -1 {
if (isLow(a) && !isLow(b)) {
return 1;
} else if (!isLow(a) && isLow(b)) {
return -1;
} else if (a > b) {
return 1;
} else if (a < b) {
return -1;
}
return 0;
}
1 change: 1 addition & 0 deletions packages/tslint-config/src/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"extends": "../../../config/tsconfig.base",
"compilerOptions": {
"lib": ["es6", "dom"],
"module": "commonjs",
"outDir": "../lib/rules"
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Make sure that this rule doesn't modify icon classes, that's handled elsewhere.
<span className"pt-icon pt-icon-folder-open" />
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Make sure that this rule doesn't modify icon classes, that's handled elsewhere.
<span className"pt-icon pt-icon-folder-open" />
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { Classes } from "@blueprintjs/core";
apt-get
at 4pt-size
`${Classes.LARGE} script-source apt-get`
`script-source ${Classes.LARGE} apt-get`
`${template} ${Classes.LARGE}`

<Button className=`${Classes.LARGE} my-class` />

classNames(Classes.BUTTON, Classes.INTENT_PRIMARY)

<pt-popover> // angular directive

Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ at 4pt-size
~~~~~~~~ [msg]

<Button className="my-class pt-large" />
~~~~~~~~ [msg]
~~~~~~~~ [msg]

classNames(Classes.BUTTON, "pt-intent-primary")
~~~~~~~~~~~~~~~~~ [msg]
~~~~~~~~~~~~~~~~~ [msg]

<pt-popover> // angular directive

Expand Down

1 comment on commit 35327b7

@blueprint-bot
Copy link

Choose a reason for hiding this comment

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

Create a fixer for classes-constants lint rule (#2969)

Previews: documentation | landing | table

Please sign in to comment.