Skip to content

Commit

Permalink
feat: Add global detection for XML Templating
Browse files Browse the repository at this point in the history
  • Loading branch information
matz3 committed Feb 10, 2025
1 parent be2f1a7 commit ff4a39b
Show file tree
Hide file tree
Showing 12 changed files with 563 additions and 118 deletions.
20 changes: 10 additions & 10 deletions src/linter/binding/BindingLinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export default class BindingLinter {
const {events} = bindingInfo;
if (events && typeof events === "object") {
for (const eventHandler of Object.values(events)) {
this.#checkForGlobalReference(eventHandler, requireDeclarations, position);
this.checkForGlobalReference(eventHandler, requireDeclarations, position);
}
}
}
Expand All @@ -36,14 +36,14 @@ export default class BindingLinter {
if (formatter) {
if (Array.isArray(formatter)) {
formatter.forEach((formatterItem) => {
this.#checkForGlobalReference(formatterItem, requireDeclarations, position);
this.checkForGlobalReference(formatterItem, requireDeclarations, position);
});
} else {
this.#checkForGlobalReference(formatter, requireDeclarations, position);
this.checkForGlobalReference(formatter, requireDeclarations, position);
}
}
if (type) {
this.#checkForGlobalReference(type, requireDeclarations, position);
this.checkForGlobalReference(type, requireDeclarations, position);
}
}

Expand All @@ -52,10 +52,10 @@ export default class BindingLinter {
this.#analyzeCommonBindingParts(bindingInfo, requireDeclarations, position);
const {factory, groupHeaderFactory, filters, sorter} = bindingInfo;
if (factory) {
this.#checkForGlobalReference(factory, requireDeclarations, position);
this.checkForGlobalReference(factory, requireDeclarations, position);
}
if (groupHeaderFactory) {
this.#checkForGlobalReference(groupHeaderFactory, requireDeclarations, position);
this.checkForGlobalReference(groupHeaderFactory, requireDeclarations, position);
}
if (filters) {
this.#analyzeFilters(filters, requireDeclarations, position);
Expand All @@ -75,7 +75,7 @@ export default class BindingLinter {
}
const {test, filters: nestedFilters, condition} = filters;
if (test) {
this.#checkForGlobalReference(test, requireDeclarations, position);
this.checkForGlobalReference(test, requireDeclarations, position);
}
if (nestedFilters) {
this.#analyzeFilters(nestedFilters, requireDeclarations, position);
Expand All @@ -95,14 +95,14 @@ export default class BindingLinter {
}
const {group, comparator} = sorter;
if (group && typeof group !== "boolean") {
this.#checkForGlobalReference(group, requireDeclarations, position);
this.checkForGlobalReference(group, requireDeclarations, position);
}
if (comparator) {
this.#checkForGlobalReference(comparator, requireDeclarations, position);
this.checkForGlobalReference(comparator, requireDeclarations, position);
}
}

#checkForGlobalReference(ref: string, requireDeclarations: RequireDeclaration[], position: PositionInfo) {
checkForGlobalReference(ref: string, requireDeclarations: RequireDeclaration[], position: PositionInfo) {
if (ref.startsWith(".")) {
// Ignore empty reference or reference to the controller (as indicated by the leading dot)
return false;
Expand Down
49 changes: 37 additions & 12 deletions src/linter/xmlTemplate/Parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import he from "he";
import ViewGenerator from "./generator/ViewGenerator.js";
import FragmentGenerator from "./generator/FragmentGenerator.js";
import JSTokenizer from "./lib/JSTokenizer.js";
import LinterContext from "../LinterContext.js";
import LinterContext, {PositionInfo} from "../LinterContext.js";
import {TranspileResult} from "../LinterContext.js";
import AbstractGenerator from "./generator/AbstractGenerator.js";
import {getLogger} from "@ui5/logger";
Expand Down Expand Up @@ -285,7 +285,7 @@ export default class Parser {
[tagNamespace, tagName] = tagName.split(":");
}

const attributes = new Set<AttributeDeclaration>();
const attributes = new Map<string, AttributeDeclaration>();
tag.attributes.forEach((attr) => {
const attrName = attr.name.value;
const attrValue = he.decode(attr.value.value);
Expand All @@ -305,7 +305,7 @@ export default class Parser {
} else if (attrName.includes(":")) {
// Namespaced attribute
const [attrNamespace, attrLocalName] = attrName.split(":");
attributes.add({
attributes.set(attrName, {
name: attrLocalName,
value: attrValue,
localNamespace: attrNamespace,
Expand All @@ -316,7 +316,7 @@ export default class Parser {
}),
});
} else {
attributes.add({
attributes.set(attrName, {
name: attrName,
value: attrValue,
start: toPosition(attr.name.start),
Expand Down Expand Up @@ -368,13 +368,7 @@ export default class Parser {
end: toPosition(tag.openEnd),
};
} else if (namespace === TEMPLATING_NAMESPACE) {
return {
kind: NodeKind.Template,
name: tagName,
namespace,
start: toPosition(tag.openStart),
end: toPosition(tag.openEnd),
};
return this._handleTemplatingNamespace(tagName, namespace, attributes, tag);
} else if (PATTERN_LIBRARY_NAMESPACES.test(namespace)) {
const lastIdx = tagName.lastIndexOf(".");
if (lastIdx !== -1) {
Expand All @@ -396,7 +390,7 @@ export default class Parser {
}

_handleUi5LibraryNamespace(
moduleName: string, namespace: Namespace, attributes: Set<AttributeDeclaration>,
moduleName: string, namespace: Namespace, attributes: Map<string, AttributeDeclaration>,
tag: SaxTag
): ControlDeclaration | AggregationDeclaration | FragmentDefinitionDeclaration {
const controlProperties = new Set<PropertyDeclaration>();
Expand Down Expand Up @@ -610,4 +604,35 @@ export default class Parser {
return node;
}
}

_handleTemplatingNamespace(
tagName: string, namespace: Namespace, attributes: Map<string, AttributeDeclaration>, tag: SaxTag
): NodeDeclaration {
let globalReferenceCheckAttribute: AttributeDeclaration | undefined;
if (tagName === "alias") {
globalReferenceCheckAttribute = attributes.get("value");
} else if (tagName === "with") {
globalReferenceCheckAttribute = attributes.get("helper");
}

if (globalReferenceCheckAttribute) {
this._checkGlobalReference(globalReferenceCheckAttribute.value, globalReferenceCheckAttribute.start);
}

return {
kind: NodeKind.Template,
name: tagName,
namespace,
start: toPosition(tag.openStart),
end: toPosition(tag.openEnd),
};
}

_checkGlobalReference(value: string, {line, column}: PositionInfo) {
this.#bindingLinter.checkForGlobalReference(value, this.#requireDeclarations, {
// Add one to align with IDEs
line: line + 1,
column: column + 1,
});
}
}
10 changes: 9 additions & 1 deletion test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -1896,10 +1896,18 @@ Generated by [AVA](https://avajs.dev).
[
{
coverageInfo: [],
errorCount: 3,
errorCount: 4,
fatalErrorCount: 0,
filePath: 'XMLTemplatingRequire.view.xml',
messages: [
{
column: 37,
line: 10,
message: 'Access of global variable \'AH\' (AH.resolvePath)',
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: 9,
line: 15,
Expand Down
Binary file modified test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.snap
Binary file not shown.
21 changes: 19 additions & 2 deletions test/lib/linter/rules/snapshots/NoGlobals.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -607,10 +607,27 @@ Generated by [AVA](https://avajs.dev).
[
{
coverageInfo: [],
errorCount: 0,
errorCount: 2,
fatalErrorCount: 0,
filePath: 'XMLTemplatingV2.view.xml',
messages: [],
messages: [
{
column: 38,
line: 12,
message: 'Access of global variable \'sap\' (sap.ui.core.sample.ViewTemplate.scenario.Helper.formatParts)',
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: 41,
line: 18,
message: 'Access of global variable \'sap\' (sap.ui.model.odata.AnnotationHelper.gotoEntityType)',
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,
},
],
warningCount: 0,
},
]
Expand Down
Binary file modified test/lib/linter/rules/snapshots/NoGlobals.ts.snap
Binary file not shown.
Loading

0 comments on commit ff4a39b

Please sign in to comment.