Skip to content

Commit

Permalink
feat: Detect inline XML fragments & views (#519)
Browse files Browse the repository at this point in the history
JIRA: CPOUI5FOUNDATION-986

---------

Co-authored-by: Matthias Osswald <[email protected]>
  • Loading branch information
d3xter666 and matz3 authored Feb 14, 2025
1 parent 45edfe0 commit e85ad26
Show file tree
Hide file tree
Showing 13 changed files with 667 additions and 10 deletions.
2 changes: 2 additions & 0 deletions src/linter/LinterContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ 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 ts from "typescript";

export type FilePattern = string; // glob patterns
export type FilePath = string; // Platform-dependent path
Expand Down Expand Up @@ -97,6 +98,7 @@ export interface LintMetadata {
directives: Set<Directive>;
transformedImports: Map<string, Set<string>>;
xmlCompiledResource: string;
jsToXmlPosMapping: {pos: ts.LineAndCharacter; originalPath: string};
}

export default class LinterContext {
Expand Down
76 changes: 74 additions & 2 deletions src/linter/ui5Types/SourceFileLinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ import {
} from "./utils.js";
import {taskStart} from "../../utils/perf.js";
import {getPositionsForNode} from "../../utils/nodePosition.js";
import {TraceMap} from "@jridgewell/trace-mapping";
import {SourceMapInput, TraceMap, originalPositionFor} from "@jridgewell/trace-mapping";
import type {ApiExtract} from "../../utils/ApiExtract.js";
import {findDirectives} from "./directives.js";
import BindingLinter from "../binding/BindingLinter.js";
import {RequireDeclaration} from "../xmlTemplate/Parser.js";
import BindingParser, {PropertyBindingInfo} from "../binding/lib/BindingParser.js";
import {createResource} from "@ui5/fs/resourceFactory";
import {AbstractAdapter} from "@ui5/fs";

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

Expand Down Expand Up @@ -67,6 +69,7 @@ export default class SourceFileLinter {
#isComponent: boolean;
#hasTestStarterFindings: boolean;
#metadata: LintMetadata;
#xmlContents: {xml: string; pos: ts.LineAndCharacter; viewType: "fragment" | "view"}[];

constructor(
private context: LinterContext,
Expand All @@ -77,6 +80,8 @@ export default class SourceFileLinter {
private reportCoverage = false,
private messageDetails = false,
private apiExtract: ApiExtract,
private filePathsWorkspace: AbstractAdapter,
private workspace: AbstractAdapter,
private manifestContent?: string
) {
this.#reporter = new SourceFileReporter(context, resourcePath,
Expand All @@ -86,6 +91,7 @@ export default class SourceFileLinter {
this.#isComponent = this.#fileName === "Component.js" || this.#fileName === "Component.ts";
this.#hasTestStarterFindings = false;
this.#metadata = this.context.getMetadata(this.resourcePath);
this.#xmlContents = [];
}

// eslint-disable-next-line @typescript-eslint/require-await
Expand All @@ -103,6 +109,21 @@ export default class SourceFileLinter {
this.#reportTestStarter(this.sourceFile);
}

let i = 0;
for (const xmlContent of this.#xmlContents) {
const fileName = `${this.sourceFile.fileName.replace(/(\.js|\.ts)$/, "")}.inline-${++i}.${xmlContent.viewType}.xml`;
const metadata = this.context.getMetadata(fileName);
const newResource = createResource({path: fileName, string: xmlContent.xml});
metadata.jsToXmlPosMapping = {
pos: xmlContent.pos,
originalPath: this.sourceFile.fileName,
};
await Promise.all([
await this.filePathsWorkspace.write(newResource),
await this.workspace.write(newResource),
]);
}

this.#reporter.deduplicateMessages();
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
Expand Down Expand Up @@ -736,7 +757,7 @@ export default class SourceFileLinter {
});
}

extractNamespace(node: ts.PropertyAccessExpression | ts.ElementAccessExpression): string {
extractNamespace(node: ts.PropertyAccessExpression | ts.ElementAccessExpression | ts.CallExpression): string {
const propAccessChain: string[] = [];
propAccessChain.push(node.expression.getText());

Expand Down Expand Up @@ -860,6 +881,20 @@ export default class SourceFileLinter {
if (this.#isPropertyBinding(node, [propName, alternativePropName])) {
this.#analyzePropertyBindings(node.arguments[0], ["type", "formatter"]);
}
} else if (["view", "xmlview", "fragment", "xmlfragment"].includes(symbolName)) {
const namespace = this.extractNamespace(node)
.replace(/\[("|'|`)*/g, ".").replace(/("|'|`)*\]/g, "");
const typeProperty = node?.arguments?.[0] && ts.isObjectLiteralExpression(node.arguments[0]) &&
getPropertyAssignmentInObjectLiteralExpression("type", node.arguments[0]);
if (namespace === `sap.ui.${symbolName}` &&
(!typeProperty ||
(ts.isStringLiteralLike(typeProperty.initializer) && typeProperty.initializer.text === "XML"))
) {
const viewType = ["fragment", "xmlfragment"].includes(symbolName) ? "fragment" : "view";
this.#extractXmlFromJs(node, viewType);
}
} else if (symbolName === "create" && moduleName === "sap/ui/core/mvc/XMLView") {
this.#extractXmlFromJs(node, "view");
}
}

Expand Down Expand Up @@ -1003,6 +1038,38 @@ export default class SourceFileLinter {
}
}

#extractXmlFromJs(node: ts.CallExpression, viewType: "view" | "fragment" = "view") {
if (!node.arguments.length || !ts.isObjectLiteralExpression(node.arguments[0]) ||
// xmlCompiledResource is an XML file, so we don't need to do extraction here
this.#metadata?.xmlCompiledResource) {
return;
}

node.arguments[0].properties.forEach((prop) => {
if (ts.isPropertyAssignment(prop) &&
(ts.isIdentifier(prop.name) || ts.isStringLiteralLike(prop.name)) &&
ts.isStringLiteralLike(prop.initializer)) {
const sourceMapJson = this.sourceMaps?.get(this.sourceFile.fileName);
const traceMap = sourceMapJson ?
new TraceMap(JSON.parse(sourceMapJson) as SourceMapInput) :
undefined;
const pos = this.sourceFile.getLineAndCharacterOfPosition(prop.initializer.pos);
const posInSource = traceMap ?
originalPositionFor(traceMap, {line: pos.line, column: pos.character}) :
null;
const originalPos = posInSource ?
{line: posInSource.line ?? 0, character: posInSource.column ?? 0} :
pos;

this.#xmlContents.push({
xml: prop.initializer.text,
pos: originalPos,
viewType,
});
}
});
}

#analyzeParametersGetCall(node: ts.CallExpression) {
if (node.arguments.length && ts.isObjectLiteralExpression(node.arguments[0])) {
// Non-deprecated usage
Expand Down Expand Up @@ -1143,6 +1210,9 @@ export default class SourceFileLinter {
typeValue,
}, node);
}
if (typeValue === "XML") {
this.#extractXmlFromJs(node, "view");
}
}
}

Expand Down Expand Up @@ -1170,6 +1240,8 @@ export default class SourceFileLinter {
typeValue,
}, node);
}
} else if (typeValue === "XML" && symbolName === "load") {
this.#extractXmlFromJs(node, "fragment");
}
}
}
Expand Down
47 changes: 43 additions & 4 deletions src/linter/ui5Types/TypeLinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import path from "node:path/posix";
import {AbstractAdapter} from "@ui5/fs";
import {createAdapter, createResource} from "@ui5/fs/resourceFactory";
import {loadApiExtract} from "../../utils/ApiExtract.js";
import {CONTROLLER_BY_ID_DTS_PATH} from "../xmlTemplate/linter.js";
import lintXml, {CONTROLLER_BY_ID_DTS_PATH} from "../xmlTemplate/linter.js";
import type SharedLanguageService from "./SharedLanguageService.js";

const log = getLogger("linter:ui5Types:TypeLinter");
Expand Down Expand Up @@ -113,11 +113,11 @@ export default class TypeChecker {
this.#sharedLanguageService.acquire(host);

const createProgramDone = taskStart("ts.createProgram", undefined, true);
const program = this.#sharedLanguageService.getProgram();
let program = this.#sharedLanguageService.getProgram();
createProgramDone();

const getTypeCheckerDone = taskStart("program.getTypeChecker", undefined, true);
const checker = program.getTypeChecker();
let checker = program.getTypeChecker();
getTypeCheckerDone();

const apiExtract = await loadApiExtract();
Expand Down Expand Up @@ -146,12 +146,51 @@ export default class TypeChecker {
this.#context, sourceFile.fileName,
sourceFile, sourceMaps,
checker, reportCoverage, messageDetails,
apiExtract, manifestContent
apiExtract, this.#filePathsWorkspace, this.#workspace, manifestContent
);
await linter.lint();
linterDone();
}
}

// Will eventually produce new JS files for XML views and fragments
await lintXml({
filePathsWorkspace: this.#filePathsWorkspace,
workspace: this.#workspace,
context: this.#context,
altGlob: "**/*.inline-*.{view,fragment}.xml",
});
const virtualXMLResources =
await this.#filePathsWorkspace.byGlob("/**/*.inline-*.{view,fragment}{.js,.ts,.js.map}");
if (virtualXMLResources.length > 0) {
for (const resource of virtualXMLResources) {
const resourcePath = resource.getPath();
if (resourcePath.endsWith(".js.map")) {
sourceMaps.set(
// Remove ".map" from path to have it reflect the associated source path
resourcePath.slice(0, -4),
await resource.getString()
);
} else {
files.set(resourcePath, await resource.getString());
}
}
program = this.#sharedLanguageService.getProgram();
checker = program.getTypeChecker();
for (const sourceFile of program.getSourceFiles()) {
if (!sourceFile.isDeclarationFile && /\.inline-[0-9]+\.(view|fragment)\.js/.exec(sourceFile.fileName)) {
const linterDone = taskStart("Type-check resource", sourceFile.fileName, true);
const linter = new SourceFileLinter(
this.#context, sourceFile.fileName,
sourceFile, sourceMaps,
checker, reportCoverage, messageDetails,
apiExtract, this.#filePathsWorkspace, this.#workspace
);
await linter.lint();
linterDone();
}
}
}
typeCheckDone();

this.#sharedLanguageService.release();
Expand Down
40 changes: 37 additions & 3 deletions src/linter/xmlTemplate/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@ import transpileXml from "./transpiler.js";
import {LinterParameters} from "../LinterContext.js";
import ControllerByIdInfo from "./ControllerByIdInfo.js";
import {ControllerByIdDtsGenerator} from "./generator/ControllerByIdDtsGenerator.js";
import {decodedMap, DecodedSourceMap, SourceMapInput, TraceMap} from "@jridgewell/trace-mapping";

// For usage in TypeLinter to write the file as part of the internal WRITE_TRANSFORMED_SOURCES debug mode
export const CONTROLLER_BY_ID_DTS_PATH = "/types/@ui5/linter/virtual/ControllerById.d.ts";

export default async function lintXml({filePathsWorkspace, workspace, context}: LinterParameters) {
const xmlResources = await filePathsWorkspace.byGlob("**/{*.view.xml,*.fragment.xml}");
export default async function lintXml({filePathsWorkspace, workspace, context, altGlob}:
LinterParameters & {altGlob?: string}) {
altGlob = altGlob ?? "**/{*.view.xml,*.fragment.xml}";
const xmlResources = await filePathsWorkspace.byGlob(altGlob);

const controllerByIdInfo = new ControllerByIdInfo();

Expand All @@ -21,6 +24,20 @@ export default async function lintXml({filePathsWorkspace, workspace, context}:
const {source, map} = res;
const resourcePath = resource.getPath();

let xmlFromJsResourceMap;
const metadata = context.getMetadata(resourcePath);
if (metadata.jsToXmlPosMapping) {
xmlFromJsResourceMap = JSON.parse(map) as DecodedSourceMap;
const {pos} = metadata.jsToXmlPosMapping;
// If it's an XML snippet extracted from a JS file, adjust the source map positions
// as these positions are relative to the extracted string, not to the real position in the JS file.
// Add that missing line shift from the original JS file to the source map.
xmlFromJsResourceMap = fixSourceMapIndices(xmlFromJsResourceMap, pos.line, pos.character);
// Replace the name of the source file in the source map with the original JS file name,
// so that reporter will lead to the original source file.
xmlFromJsResourceMap.sources.splice(0, 1, metadata.jsToXmlPosMapping.originalPath.split("/").pop() ?? null);
}

// Write transpiled resource to workspace
// TODO: suffix name to prevent clashes with existing files?
const jsPath = resourcePath.replace(/\.xml$/, ".js");
Expand All @@ -30,7 +47,7 @@ export default async function lintXml({filePathsWorkspace, workspace, context}:
});
const transpiledResourceSourceMap = createResource({
path: jsPath + ".map",
string: map,
string: xmlFromJsResourceMap ? JSON.stringify(xmlFromJsResourceMap) : map,
});

await filePathsWorkspace.write(transpiledResource);
Expand All @@ -54,3 +71,20 @@ export default async function lintXml({filePathsWorkspace, workspace, context}:
await workspace.write(dtsResource);
}
}

function fixSourceMapIndices(map: SourceMapInput, lineShift = 0, columnShift = 0): DecodedSourceMap {
const decodedTraceMap = decodedMap(new TraceMap(map));
const {mappings} = decodedTraceMap;

const newMappings = mappings.map((segment) =>
segment.map(([genCol, srcIndex, origLine, origCol, nameIndex]) => {
const calculatedColShift = (origLine === 0 ? columnShift : 0);
return (srcIndex !== undefined ?
[genCol + calculatedColShift, srcIndex, origLine! + lineShift,
origCol! + calculatedColShift, nameIndex ?? 0] :
[genCol + calculatedColShift]);
})
);

return {...decodedTraceMap, mappings: newMappings as DecodedSourceMap["mappings"]};
}
29 changes: 29 additions & 0 deletions test/fixtures/linter/rules/XmlInJs/FragmentLoad.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
sap.ui.require(["sap/ui/core/Fragment"], async (Fragment) => {
const content = await Fragment.load({
type: "XML",
"definition": `<core:FragmentDefinition
xmlns="sap.m"
xmlns:core="sap.ui.core">
<Button tap=".sayHello">
</core:FragmentDefinition>`,
});

const content2 = await Fragment["load"]({
type: "XML",
definition: `<core:FragmentDefinition
xmlns="sap.m"
xmlns:core="sap.ui.core">
<Button tap=".sayHello">
</core:FragmentDefinition>`,
});

const content3 = await Fragment.load({
type: "HTML", // Should be ignored as analysis is only for XML fragments
// Intentionally put XML content, so it must be skipped for compilation & analysis
definition: `<core:FragmentDefinition
xmlns="sap.m"
xmlns:core="sap.ui.core">
<Button tap=".sayHello">
</core:FragmentDefinition>`
});
});
36 changes: 36 additions & 0 deletions test/fixtures/linter/rules/XmlInJs/SapUiFragment.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
sap.ui.require([], () => {
const fragment = sap.ui.fragment({
type: "XML",
fragmentContent: `<core:FragmentDefinition
xmlns="sap.m"
xmlns:core="sap.ui.core">
<Button tap=".sayHello">
</core:FragmentDefinition>`,
});

const fragment2 = sap.ui.xmlfragment({
"fragmentContent": `<core:FragmentDefinition
xmlns="sap.m"
xmlns:core="sap.ui.core">
<Button tap=".sayHello">
</core:FragmentDefinition>`,
});

const fragment3 = sap["ui"].xmlfragment({
"fragmentContent": `<core:FragmentDefinition
xmlns="sap.m"
xmlns:core="sap.ui.core">
<Button tap=".sayHello">
</core:FragmentDefinition>`,
});

const fragment4 = sap.ui.fragment({
type: "HTML", // Should be ignored as analysis is only for XML fragments
// Intentionally put XML content, so it must be skipped for compilation & analysis
fragmentContent: `<core:FragmentDefinition
xmlns="sap.m"
xmlns:core="sap.ui.core">
<Button tap=".sayHello">
</core:FragmentDefinition>`,
});
});
Loading

0 comments on commit e85ad26

Please sign in to comment.