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 29, 2024
1 parent e316ef0 commit 4063044
Show file tree
Hide file tree
Showing 10 changed files with 158 additions and 32 deletions.
3 changes: 2 additions & 1 deletion src/linter/LinterContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ 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";

export type FilePattern = string; // glob patterns
export type FilePath = string; // Platform-dependent path
Expand Down Expand Up @@ -94,7 +95,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
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
98 changes: 98 additions & 0 deletions src/linter/ui5Types/directives.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
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() && directive.pos + directive.length <= node.getEnd()) {
const trailingComments = ts.getTrailingCommentRanges(sourceText, node.getEnd());
if (trailingComments?.length) {
trailingComments.some((comment) => {
if (comment.pos === directive.pos) {
possibleDirectives.delete(directive);
confirmedDirectives.add(directive);
return true;
}
return false;
});
break;
}
}
}
}

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

export interface Directive {
action: string;
isLine: boolean;
isNextLine: boolean;
ruleNames: string[];
pos: number;
length: 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;
const ruleNames = rules?.trim().split(",") ?? [];
const isLine = nextLineOrLine === "-line";
const isNextLine = nextLineOrLine === "-next-line";
comments.add({action, isLine, isNextLine, ruleNames, pos, length});
}
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
7 changes: 7 additions & 0 deletions test/fixtures/linter/rules/Directives/Directives.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,11 @@ sap.ui.define([
blocked: true, // REPORT: Property "blocked" is deprecated
tap: () => console.log("Tapped") // REPORT: Event "tap" is deprecated
});
return;
// ui5lint-disable
});

new sap.m.Button({ // IGNORE: Global variable "sap"
blocked: true, // IGNORE: Property "blocked" is deprecated
tap: () => console.log("Tapped") // IGNORE: Event "tap" is deprecated
});
16 changes: 9 additions & 7 deletions test/lib/linter/rules/snapshots/Directives.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,47 +140,47 @@ Generated by [AVA](https://avajs.dev).
line: 51,
message: 'Use of deprecated property \'webview\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-property',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 2,
line: 53,
message: 'Use of deprecated property \'webview\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-property',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 2,
line: 55,
message: 'Use of deprecated property \'webview\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-property',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 2,
line: 58,
message: 'Use of deprecated property \'AnimationMode\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-property',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 2,
line: 61,
message: 'Use of deprecated property \'MessageType\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-property',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 2,
line: 64,
message: 'Use of deprecated property \'MessageType\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-property',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
Expand Down Expand Up @@ -220,7 +220,7 @@ Generated by [AVA](https://avajs.dev).
line: 79,
message: 'Use of deprecated property \'Date\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-property',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
Expand All @@ -235,13 +235,15 @@ Generated by [AVA](https://avajs.dev).
column: 6,
line: 87,
message: 'Access of global variable \'sap\' (sap.m.Button)',
messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
ruleId: 'no-globals',
severity: 2,
},
{
column: 6,
line: 93,
message: 'Access of global variable \'sap\' (sap.m.Button)',
messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
ruleId: 'no-globals',
severity: 2,
},
Expand Down
Binary file modified test/lib/linter/rules/snapshots/Directives.ts.snap
Binary file not shown.

0 comments on commit 4063044

Please sign in to comment.