Skip to content

Commit

Permalink
feat: Add ui5-lint-disable directives
Browse files Browse the repository at this point in the history
  • Loading branch information
RandomByte committed Oct 31, 2024
1 parent e316ef0 commit 04cf2de
Show file tree
Hide file tree
Showing 10 changed files with 275 additions and 245 deletions.
81 changes: 79 additions & 2 deletions src/linter/LinterContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import {createReader} from "@ui5/fs/resourceFactory";
import {resolveLinks} from "../formatter/lib/resolveLinks.js";
import {LintMessageSeverity, MESSAGE, MESSAGE_INFO} from "./messages.js";
import {MessageArgs} from "./MessageArgs.js";
import {Directive} from "./ui5Types/directives.js";
import {filter} from "minimatch";

Check failure on line 7 in src/linter/LinterContext.ts

View workflow job for this annotation

GitHub Actions / General checks, tests and coverage reporting

'filter' is defined but never used. Allowed unused vars must match /^_/u

export type FilePattern = string; // glob patterns
export type FilePath = string; // Platform-dependent path
Expand Down Expand Up @@ -94,7 +96,7 @@ export interface LintMetadata {
// TODO: Use this to store information shared across linters,
// such as the async flag state in manifest.json which might be relevant
// when parsing the Component.js
_todo: string;
directives: Set<Directive>;
}

export default class LinterContext {
Expand Down Expand Up @@ -237,11 +239,86 @@ export default class LinterContext {
generateLintResult(resourcePath: ResourcePath): LintResult {
const rawMessages = this.#rawMessages.get(resourcePath) ?? [];
const coverageInfo = this.#coverageInfo.get(resourcePath) ?? [];
const metadata = this.#metadata.get(resourcePath);
let errorCount = 0;
let warningCount = 0;
let fatalErrorCount = 0;

const messages: LintMessage[] = rawMessages.map((rawMessage) => {
let filteredMessages: RawLintMessage[] = rawMessages;
if (metadata?.directives?.size) {
filteredMessages = [];
const directives = new Set(metadata.directives);
// Sort messages by position
const sortedMessages = rawMessages.filter((rawMessage) => {
if (!rawMessage.position) {
filteredMessages.push(rawMessage);
return false;
}
return true;
}).sort((a, b) => {
const aPos = a.position!;
const bPos = b.position!;
return aPos.line === bPos.line ? aPos.column - bPos.column : aPos.line - bPos.line;
});

// Filter messages based on directives
let directiveStack: Directive[] = [];
for (const rawMessage of sortedMessages) {
const {position} = rawMessage;
const {line, column} = position!;

Check failure on line 268 in src/linter/LinterContext.ts

View workflow job for this annotation

GitHub Actions / General checks, tests and coverage reporting

'column' is assigned a value but never used. Allowed unused vars must match /^_/u

directiveStack = directiveStack.filter((dir) => {
// Filter out line-based directives that are no longer relevant
if (dir.isLine && dir.line !== line) {
return false;
}
if (dir.isNextLine && dir.line !== line - 1) {
return false;
}
return true;
});

for (const dir of directives) {
if (dir.line > line) {
continue;
}
// if (dir.isLine && dir.line !== line) {
// continue;
// }
// if (dir.isNextLine && dir.line !== line - 1) {
// continue;
// }
directives.delete(dir);
directiveStack.push(dir);
}

if (!directiveStack.length) {
filteredMessages.push(rawMessage);
continue;
}

const messageInfo = MESSAGE_INFO[rawMessage.id];
if (!messageInfo) {
throw new Error(`Invalid message id '${rawMessage.id}'`);
}

let disabled = false;
for (const dir of directiveStack) {
if (dir.action === "disable" &&
(!dir.ruleNames.length || dir.ruleNames.includes(messageInfo.ruleId))) {
disabled = true;
} else if (dir.action === "enable" &&
(!dir.ruleNames.length || dir.ruleNames.includes(messageInfo.ruleId))) {
disabled = false;
}
}
if (!disabled) {
filteredMessages.push(rawMessage);
}
}
}

const messages: LintMessage[] = filteredMessages.map((rawMessage) => {
const message = this.#getMessageFromRawMessage(rawMessage);
if (message.severity === LintMessageSeverity.Error) {
errorCount++;
Expand Down
7 changes: 7 additions & 0 deletions src/linter/ui5Types/SourceFileLinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {getPropertyName} from "./utils.js";
import {taskStart} from "../../utils/perf.js";
import {getPositionsForNode} from "../../utils/nodePosition.js";
import {TraceMap} from "@jridgewell/trace-mapping";
import {findDirectives} from "./directives.js";

const log = getLogger("linter:ui5Types:SourceFileLinter");

Expand Down Expand Up @@ -81,6 +82,12 @@ export default class SourceFileLinter {
// eslint-disable-next-line @typescript-eslint/require-await
async lint() {
try {
const metadata = this.#context.getMetadata(this.#resourcePath);
if (!metadata.directives) {
// Directives might have already been extracted by the amd transpiler
// This is done since the transpile process might loose comments
findDirectives(this.#sourceFile, metadata);
}
this.visitNode(this.#sourceFile);
this.#reporter.deduplicateMessages();
} catch (err) {
Expand Down
2 changes: 1 addition & 1 deletion src/linter/ui5Types/TypeLinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export default class TypeChecker {
}
}

const host = await createVirtualCompilerHost(this.#compilerOptions, files, sourceMaps);
const host = await createVirtualCompilerHost(this.#compilerOptions, files, sourceMaps, this.#context);

const createProgramDone = taskStart("ts.createProgram", undefined, true);
const program = ts.createProgram(pathsToLint, this.#compilerOptions, host);
Expand Down
11 changes: 7 additions & 4 deletions src/linter/ui5Types/amdTranspiler/transpiler.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import ts from "typescript";
import path from "node:path/posix";
import {getLogger} from "@ui5/logger";
import {taskStart} from "../../../utils/perf.js";
import {TranspileResult} from "../../LinterContext.js";
import LinterContext, {TranspileResult} from "../../LinterContext.js";
import {createTransformer} from "./tsTransformer.js";
import {UnsupportedModuleError} from "./util.js";

Expand Down Expand Up @@ -49,9 +50,11 @@ function createProgram(inputFileNames: string[], host: ts.CompilerHost): ts.Prog
return ts.createProgram(inputFileNames, compilerOptions, host);
}

export default function transpileAmdToEsm(fileName: string, content: string, strict?: boolean): TranspileResult {
export default function transpileAmdToEsm(
resourcePath: string, content: string, context: LinterContext, strict?: boolean
): TranspileResult {
// This is heavily inspired by the TypesScript "transpileModule" API

const fileName = path.basename(resourcePath);
const taskDone = taskStart("Transpiling AMD to ESM", fileName, true);
const sourceFile = ts.createSourceFile(
fileName,
Expand All @@ -71,7 +74,7 @@ export default function transpileAmdToEsm(fileName: string, content: string, str
const program = createProgram([fileName], compilerHost);

const transformers: ts.CustomTransformers = {
before: [createTransformer(program)],
before: [createTransformer(program, resourcePath, context)],
};

try {
Expand Down
23 changes: 15 additions & 8 deletions src/linter/ui5Types/amdTranspiler/tsTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import replaceNodeInParent, {NodeReplacement} from "./replaceNodeInParent.js";
import {toPosStr, UnsupportedModuleError} from "./util.js";
import rewriteExtendCall, {UnsupportedExtendCall} from "./rewriteExtendCall.js";
import insertNodesInParent from "./insertNodesInParent.js";
import LinterContext from "../../LinterContext.js";
import {findDirectives} from "../directives.js";

const log = getLogger("linter:ui5Types:amdTranspiler:TsTransformer");

Expand Down Expand Up @@ -44,21 +46,23 @@ function isBlockLike(node: ts.Node): node is ts.BlockLike {
* error is thrown. In that case, the rest of the module is still processed. However it's possible that the result
* will be equal to the input.
*/
export function createTransformer(program: ts.Program): ts.TransformerFactory<ts.SourceFile> {
return function transformer(context: ts.TransformationContext) {
export function createTransformer(
program: ts.Program, resourcePath: string, context: LinterContext
): ts.TransformerFactory<ts.SourceFile> {
return function transformer(tContext: ts.TransformationContext) {
return (sourceFile: ts.SourceFile): ts.SourceFile => {
return transform(program, sourceFile, context);
return transform(program, sourceFile, tContext, resourcePath, context);
};
};
}

function transform(
program: ts.Program, sourceFile: ts.SourceFile, context: ts.TransformationContext
program: ts.Program, sourceFile: ts.SourceFile, tContext: ts.TransformationContext, resourcePath: string,
context: LinterContext
): ts.SourceFile {
const resourcePath = sourceFile.fileName;
log.verbose(`Transforming ${resourcePath}`);
const checker = program.getTypeChecker();
const {factory: nodeFactory} = context;
const {factory: nodeFactory} = tContext;
const moduleDefinitions: ModuleDefinition[] = [];
// TODO: Filter duplicate imports, maybe group by module definition
const requireImports: ts.ImportDeclaration[] = [];
Expand Down Expand Up @@ -96,9 +100,12 @@ function transform(
insertions.push(nodeToBeInserted);
}

const metadata = context.getMetadata(resourcePath);
findDirectives(sourceFile, metadata);

// Visit the AST depth-first and collect module definitions
function visit(nodeIn: ts.Node): ts.VisitResult<ts.Node> {
const node = ts.visitEachChild(nodeIn, visit, context);
const node = ts.visitEachChild(nodeIn, visit, tContext);
if (ts.isCallExpression(node) &&
ts.isPropertyAccessExpression(node.expression)) {
if (matchPropertyAccessExpression(node.expression, "sap.ui.define")) {
Expand Down Expand Up @@ -346,7 +353,7 @@ function transform(
node = replaceNodeInParent(node, replacement, nodeFactory);
}
}
return ts.visitEachChild(node, applyModifications, context);
return ts.visitEachChild(node, applyModifications, tContext);
}
processedSourceFile = ts.visitNode(processedSourceFile, applyModifications) as ts.SourceFile;

Expand Down
114 changes: 114 additions & 0 deletions src/linter/ui5Types/directives.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import ts from "typescript";
import {LintMetadata} from "../LinterContext.js";

export function findDirectives(sourceFile: ts.SourceFile, metadata: LintMetadata) {
metadata.directives = new Set<Directive>();

const possibleDirectives = collectPossibleDirectives(sourceFile);
if (possibleDirectives.size === 0) {
return;
}
const sourceText = sourceFile.getFullText();

traverseAndFindDirectives(sourceFile, sourceText, possibleDirectives, metadata.directives);
}

function traverseAndFindDirectives(
node: ts.Node, sourceText: string, possibleDirectives: Set<Directive>, confirmedDirectives: Set<Directive>
) {
findDirectivesAroundNode(node, sourceText, possibleDirectives, confirmedDirectives);
node.getChildren().forEach((child) => {
traverseAndFindDirectives(child, sourceText, possibleDirectives, confirmedDirectives);
});
}

export function findDirectivesAroundNode(
node: ts.Node, sourceText: string, possibleDirectives: Set<Directive>, confirmedDirectives: Set<Directive>
) {
/*
// This is a comment
// ui5-lint-disable
myCallExpression()
// ui5-lint-enable
// This is a comment
*/
for (const directive of possibleDirectives) {
if (directive.pos >= node.getFullStart() && directive.pos + directive.length < node.getStart()) {
const leadingComments = ts.getLeadingCommentRanges(sourceText, node.getFullStart());
if (leadingComments?.length) {
leadingComments.some((comment) => {
if (comment.pos === directive.pos) {
possibleDirectives.delete(directive);
confirmedDirectives.add(directive);
return true;
}
return false;
});
break;
}
} else if (directive.pos > node.getEnd()) {
const trailingComments = ts.getTrailingCommentRanges(sourceText, node.getEnd());
if (trailingComments?.length) {
trailingComments.some((comment) => {
if (comment.pos === directive.pos && comment.end === directive.pos + directive.length) {
possibleDirectives.delete(directive);
confirmedDirectives.add(directive);
return true;
}
return false;
});
break;
}
}
}
}

/* Match things like:
// ui5lint-disable-next-line no-deprecated-api, no-global
// ui5lint-enable-next-line
// ui5lint-enable-line
// ui5lint-enable-line -- my comment
/* ui5lint-enable-line -- my comment *\/
/* ui5lint-disable
no-deprecated-api,
no-global
*\/
*/
const disableCommentRegex = /\/[/*]\s*ui5lint-(enable|disable)((?:-next)?-line)?((?:\s+[\w-]+,)*(?:\s+[\w-]+))?\s*(?:--.*)?(?:\*\/|$)/mg;

export interface Directive {
action: "enable" | "disable";
isLine: boolean;
isNextLine: boolean;
ruleNames: string[];
pos: number;
length: number;
line: number;
column: number;
}

export function collectPossibleDirectives(sourceFile: ts.SourceFile) {
const text = sourceFile.getFullText();
let match;
const comments = new Set<Directive>();
while ((match = disableCommentRegex.exec(text)) !== null) {
const [, action, nextLineOrLine, rules] = match;
const pos = match.index;
const length = match[0].length;
let ruleNames = rules?.split(",") ?? [];
ruleNames = ruleNames.map((rule) => rule.trim());
const isLine = nextLineOrLine === "-line";
const isNextLine = nextLineOrLine === "-next-line";
const {line, character: column} = sourceFile.getLineAndCharacterOfPosition(pos);

comments.add({
action: action as "enable" | "disable",
isLine, isNextLine, ruleNames,
pos, length,
// Typescript positions are all zero-based
line: line + 1,
column: column + 1,
});
}
return comments;
}
23 changes: 12 additions & 11 deletions src/linter/ui5Types/host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import posixPath from "node:path/posix";
import fs from "node:fs/promises";
import {createRequire} from "node:module";
import transpileAmdToEsm from "./amdTranspiler/transpiler.js";
import {ResourcePath} from "../LinterContext.js";
import LinterContext, {ResourcePath} from "../LinterContext.js";
import {getLogger} from "@ui5/logger";
const log = getLogger("linter:ui5Types:host");
const require = createRequire(import.meta.url);
Expand Down Expand Up @@ -65,7 +65,8 @@ export type FileContents = Map<ResourcePath, string | (() => string)>;

export async function createVirtualCompilerHost(
options: ts.CompilerOptions,
files: FileContents, sourceMaps: FileContents
files: FileContents, sourceMaps: FileContents,
context: LinterContext
): Promise<ts.CompilerHost> {
const silly = log.isLevelEnabled("silly");

Expand Down Expand Up @@ -113,25 +114,25 @@ export async function createVirtualCompilerHost(
}
}

function getFile(fileName: string): string | undefined {
function getFile(resourcePath: string): string | undefined {
// NOTE: This function should be kept in sync with "fileExists"

if (files.has(fileName)) {
let fileContent = files.get(fileName);
if (files.has(resourcePath)) {
let fileContent = files.get(resourcePath);
if (typeof fileContent === "function") {
fileContent = fileContent();
}
if (fileContent && fileName.endsWith(".js") && !sourceMaps.get(fileName)) {
if (fileContent && resourcePath.endsWith(".js") && !sourceMaps.get(resourcePath)) {
// No source map indicates no transpilation was done yet
const res = transpileAmdToEsm(path.basename(fileName), fileContent);
files.set(fileName, res.source);
sourceMaps.set(fileName, res.map);
const res = transpileAmdToEsm(resourcePath, fileContent, context);
files.set(resourcePath, res.source);
sourceMaps.set(resourcePath, res.map);
fileContent = res.source;
}
return fileContent;
}
if (fileName.startsWith("/types/")) {
const fsPath = mapToTypePath(fileName);
if (resourcePath.startsWith("/types/")) {
const fsPath = mapToTypePath(resourcePath);
if (fsPath) {
return ts.sys.readFile(fsPath);
}
Expand Down
Loading

0 comments on commit 04cf2de

Please sign in to comment.