Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
Extensible Rome (#1163)
Browse files Browse the repository at this point in the history
  • Loading branch information
sebmck authored Aug 31, 2020
1 parent 53a5b3e commit 97653ef
Show file tree
Hide file tree
Showing 19 changed files with 766 additions and 196 deletions.
1 change: 1 addition & 0 deletions internal/compiler/api/analyzeDependencies.test.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Object {
]
diagnostics: Array [
Object {
tags: undefined
origins: Array [Object {category: "analyzeDependencies"}]
location: Object {
filename: "unknown"
Expand Down
3 changes: 2 additions & 1 deletion internal/compiler/api/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const compileCache: Cache<CompileResult> = new Cache();
export default async function compile(
req: CompileRequest,
): Promise<CompileResult> {
const {sourceText, ast} = req;
const {sourceText, ast, project} = req;

const query = Cache.buildQuery(req);
const cached: undefined | CompileResult = compileCache.get(query);
Expand All @@ -44,6 +44,7 @@ export default async function compile(
const formatted = formatAST(
transformedAst,
{
projectConfig: project.config,
typeAnnotations: false,
indent: req.stage === "compileForBundle" ? 1 : 0,
sourceMaps: true,
Expand Down
4 changes: 3 additions & 1 deletion internal/compiler/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ export {default as Path} from "./lib/Path";
export {default as Record} from "./lib/Record";
export {default as Cache} from "./lib/Cache";

// methods
export * from "./lint/decisions";
export {default as lint} from "./lint/index";
export {LintRuleName, lintRuleNames} from "./lint/rules/index";

export {default as compile} from "./api/compile";

export {
default as analyzeDependencies,
mergeAnalyzeDependencies,
Expand Down
12 changes: 11 additions & 1 deletion internal/compiler/lib/CompilerContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,9 +344,19 @@ export default class CompilerContext {
);
}

const {marker, ...diag} = contextDiag;
let {marker, tags, ...diag} = contextDiag;

// Only set `fixable` if formatting is enabled
if (tags !== undefined && tags.fixable) {
tags = {
...tags,
fixable: this.project.config.format.enabled,
};
}

const diagnostic = this.diagnostics.addDiagnostic({
...diag,
tags,
description: {
...description,
advice,
Expand Down
103 changes: 54 additions & 49 deletions internal/compiler/lib/Path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ export default class Path {
const {category} = description;
const advice = [...description.advice];
const loc = context.getLoc(target);
const canFormat = this.context.project.config.format.enabled;

let fixed: undefined | ExitSignal = defaultFixed;

Expand All @@ -311,36 +312,38 @@ export default class Path {
),
});

if (loc === undefined) {
advice.push({
type: "log",
category: "error",
text: markup`Unable to find target location`,
});
} else {
advice.push(
buildLintDecisionAdviceAction({
filename: context.displayFilename,
decision: buildLintDecisionString({
action: "fix",
if (canFormat) {
if (loc === undefined) {
advice.push({
type: "log",
category: "error",
text: markup`Unable to find target location`,
});
} else {
advice.push(
buildLintDecisionAdviceAction({
filename: context.displayFilename,
category,
start: loc.start,
decision: buildLintDecisionString({
action: "fix",
filename: context.displayFilename,
category,
start: loc.start,
}),
shortcut: "f",
noun: markup`Apply fix`,
instruction: markup`To apply this fix run`,
}),
shortcut: "f",
noun: markup`Apply fix`,
instruction: markup`To apply this fix run`,
}),
);
);

advice.push(
buildLintDecisionAdviceAction({
extra: true,
noun: markup`Apply fix for ALL files with this category`,
instruction: markup`To apply fix for ALL files with this category run`,
decision: buildLintDecisionGlobalString("fix", category),
}),
);
advice.push(
buildLintDecisionAdviceAction({
extra: true,
noun: markup`Apply fix for ALL files with this category`,
instruction: markup`To apply fix for ALL files with this category run`,
decision: buildLintDecisionGlobalString("fix", category),
}),
);
}
}
}

Expand Down Expand Up @@ -392,30 +395,32 @@ export default class Path {
text: suggestion.description,
});

if (loc === undefined) {
advice.push({
type: "log",
category: "error",
text: markup`Unable to find target location`,
});
} else {
advice.push(
buildLintDecisionAdviceAction({
noun: suggestions.length === 1
? markup`Apply suggested fix`
: markup`Apply suggested fix "${suggestion.title}"`,
shortcut: String(num),
instruction: markup`To apply this fix run`,
filename: context.displayFilename,
decision: buildLintDecisionString({
if (canFormat) {
if (loc === undefined) {
advice.push({
type: "log",
category: "error",
text: markup`Unable to find target location`,
});
} else {
advice.push(
buildLintDecisionAdviceAction({
noun: suggestions.length === 1
? markup`Apply suggested fix`
: markup`Apply suggested fix "${suggestion.title}"`,
shortcut: String(num),
instruction: markup`To apply this fix run`,
filename: context.displayFilename,
action: "fix",
category,
start: loc.start,
id: index,
decision: buildLintDecisionString({
filename: context.displayFilename,
action: "fix",
category,
start: loc.start,
id: index,
}),
}),
}),
);
);
}
}

index++;
Expand Down
98 changes: 98 additions & 0 deletions internal/compiler/lint/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import {test} from "rome";
import {LintRequest, LintResult, lint, lintRuleNames} from "@internal/compiler";
import {ProjectConfig, createDefaultProjectConfig} from "@internal/project";
import {parseJS} from "@internal/js-parser";

function createLintTransformOptions(
sourceText: string,
mutateConfig: (config: ProjectConfig) => ProjectConfig,
): LintRequest {
return {
applySafeFixes: false,
suppressionExplanation: "",
sourceText,
ast: parseJS({
path: "unknown",
input: sourceText,
}),
options: {},
project: {
config: mutateConfig(createDefaultProjectConfig()),
directory: undefined,
},
};
}

test(
"disabledLintRules single",
async (t) => {
function hasUndeclaredDiag(res: LintResult): boolean {
for (const diag of res.diagnostics) {
if (diag.description.category === "lint/js/noUndeclaredVariables") {
return true;
}
}
return false;
}

// Make sure when it's not disabled the diagnostic is present
const res = await lint(
createLintTransformOptions("foo;", (config) => config),
);
t.true(hasUndeclaredDiag(res));

// Make sure when it's not disabled the diagnostic it is not present
const res2 = await lint(
createLintTransformOptions(
"foo;",
(config) => ({
...config,
lint: {
...config.lint,
disabledRules: ["js/noUndeclaredVariables"],
},
}),
),
);
t.false(hasUndeclaredDiag(res2));
},
);

test(
"disabledLintRules all rules",
async (t) => {
const res = await lint(
createLintTransformOptions(
"foo;",
(config) => ({
...config,
lint: {
...config.lint,
disabledRules: lintRuleNames,
},
}),
),
);
t.is(res.diagnostics.length, 0);
},
);

test(
"format disabled",
async (t) => {
const code = "wacky\n\tformatting( yes,\nok );";
const res = await lint(
createLintTransformOptions(
code,
(config) => ({
...config,
format: {
...config.format,
enabled: false,
},
}),
),
);
t.is(res.src, code);
},
);
53 changes: 48 additions & 5 deletions internal/compiler/lint/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,48 @@
*/

import {DiagnosticSuppressions, Diagnostics} from "@internal/diagnostics";
import {LintRequest} from "../types";
import {AnyVisitor, LintRequest} from "../types";
import {Cache, CompilerContext} from "@internal/compiler";
import {formatAST} from "@internal/formatter";
import {addSuppressions} from "./suppressions";
import {lintTransforms} from "./rules/index";
import {ProjectConfig} from "@internal/project";

export type LintResult = {
diagnostics: Diagnostics;
suppressions: DiagnosticSuppressions;
src: string;
};

const ruleVisitorCache: WeakMap<ProjectConfig, Array<AnyVisitor>> = new WeakMap();

const allVisitors = Array.from(lintTransforms.values());

function getVisitors(config: ProjectConfig): Array<AnyVisitor> {
const {disabledRules} = config.lint;

// Fast path
if (disabledRules.length === 0) {
return allVisitors;
}

const cached = ruleVisitorCache.get(config);
if (cached !== undefined) {
return cached;
}

const visitors: Array<AnyVisitor> = [];
ruleVisitorCache.set(config, visitors);

for (const [ruleName, visitor] of lintTransforms) {
if (!disabledRules.includes(ruleName)) {
visitors.push(visitor);
}
}

return visitors;
}

const lintCache: Cache<LintResult> = new Cache();

export default async function lint(req: LintRequest): Promise<LintResult> {
Expand All @@ -29,9 +59,12 @@ export default async function lint(req: LintRequest): Promise<LintResult> {
return cached;
}

const shouldFormat = project.config.format.enabled;
const visitors = getVisitors(project.config);

// Perform fixes
let formatAst = ast;
if (applySafeFixes) {
if (shouldFormat && applySafeFixes) {
const formatContext = new CompilerContext({
ref: req.ref,
options,
Expand All @@ -43,14 +76,24 @@ export default async function lint(req: LintRequest): Promise<LintResult> {
},
});

formatAst = formatContext.reduceRoot(lintTransforms);
formatAst = formatContext.reduceRoot(visitors);
formatAst = addSuppressions(
formatContext,
formatAst,
suppressionExplanation,
);
}
const formattedCode = formatAST(formatAst).code;

let formattedCode = req.sourceText;

if (shouldFormat) {
formattedCode = formatAST(
formatAst,
{
projectConfig: project.config,
},
).code;
}

// Run lints (could be with the autofixed AST)
const context = new CompilerContext({
Expand All @@ -63,7 +106,7 @@ export default async function lint(req: LintRequest): Promise<LintResult> {
},
frozen: true,
});
const newAst = context.reduceRoot(lintTransforms);
const newAst = context.reduceRoot(visitors);
if (ast !== newAst) {
throw new Error("Expected the same ast. `frozen: true` is broken");
}
Expand Down
Loading

0 comments on commit 97653ef

Please sign in to comment.