Skip to content

Commit 05c9b6d

Browse files
feat: Detect UI5 project specific globals (#1042)
Detects global access to project-namespace modules which bypass sap.ui.define/sap.ui.require. These were silently ignored by no-globals since app namespaces have no type definitions. This new detection generates linter messages with `ruleId`s "no-project-globals". JIRA: CPOUI5FOUNDATION-1242 --------- Co-authored-by: Max Reichmann <max.reichmann@sap.com>
1 parent 388d2de commit 05c9b6d

9 files changed

Lines changed: 231 additions & 1 deletion

File tree

docs/Rules.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
- [no-deprecated-library](#no-deprecated-library)
1111
- [no-deprecated-theme](#no-deprecated-theme)
1212
- [no-globals](#no-globals)
13+
- [no-project-globals](#no-project-globals)
1314
- [no-implicit-globals](#no-implicit-globals)
1415
- [no-pseudo-modules](#no-pseudo-modules)
1516
- [parsing-error](#parsing-error)
@@ -83,6 +84,13 @@ Checks for the usage of global variables in the code.
8384
**Related information**
8485
- [Best Practices for Developers](https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)
8586

87+
## no-project-globals
88+
89+
Checks for the usage of global variables for project-namespace modules in the code.
90+
91+
**Related information**
92+
- [Best Practices for Developers](https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)
93+
8694
## no-implicit-globals
8795

8896
Checks whether:

src/linter/messages.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ export const RULES = {
1111
"no-deprecated-library": "no-deprecated-library",
1212
"no-deprecated-theme": "no-deprecated-theme",
1313
"no-globals": "no-globals",
14+
"no-project-globals": "no-project-globals",
1415
"no-implicit-globals": "no-implicit-globals",
1516
"no-pseudo-modules": "no-pseudo-modules",
1617
"parsing-error": "parsing-error",
@@ -66,6 +67,7 @@ export enum MESSAGE {
6667
NO_DIRECT_ENUM_ACCESS,
6768
NO_EXPORTED_VALUES_BY_LIB,
6869
NO_GLOBALS,
70+
NO_PROJECT_GLOBALS,
6971
NO_ICON_POOL_RENDERER,
7072
NO_LEGACY_TEMPLATE_REQUIRE_SYNTAX,
7173
NO_LEGACY_UI5_VERSION_IN_MANIFEST,
@@ -468,6 +470,18 @@ export const MESSAGE_INFO = {
468470
`{@link topic:28fcd55b04654977b63dacbee0552712 See Best Practices for Developers}`,
469471
},
470472

473+
[MESSAGE.NO_PROJECT_GLOBALS]: {
474+
severity: LintMessageSeverity.Error,
475+
ruleId: RULES["no-project-globals"],
476+
477+
message: ({variableName, namespace}: {variableName: string; namespace: string}) =>
478+
`Access of global variable '${variableName}' (${namespace})`,
479+
details: () =>
480+
`Do not use global variables to access UI5 projects. ` +
481+
`It might be necessary to modify the provider and consumer of this global variable. ` +
482+
`{@link topic:28fcd55b04654977b63dacbee0552712 See Best Practices for Developers}`,
483+
},
484+
471485
[MESSAGE.PARSING_ERROR]: {
472486
severity: LintMessageSeverity.Error,
473487
ruleId: RULES["parsing-error"],

src/linter/ui5Types/SourceFileLinter.ts

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ export default class SourceFileLinter {
8181
#hasTestStarterFindings: boolean;
8282
#metadata: LintMetadata;
8383
#xmlContents: {xml: string; pos: ts.LineAndCharacter; documentKind: "fragment" | "view"}[];
84+
#projectNamespaceFirstSegment: string | undefined;
85+
#projectNamespaceDots: string | undefined;
8486

8587
private fixHelpers: FixHelpers;
8688
private resourcePath: ResourcePath;
@@ -113,6 +115,12 @@ export default class SourceFileLinter {
113115
manifestContent: this.manifestContent,
114116
libraryDependencies: this.libraryDependencies,
115117
};
118+
119+
const namespace = this.typeLinter.getContext().getNamespace();
120+
if (namespace) {
121+
this.#projectNamespaceDots = namespace.replace(/\//g, ".");
122+
this.#projectNamespaceFirstSegment = this.#projectNamespaceDots.split(".")[0];
123+
}
116124
}
117125

118126
async lint() {
@@ -1623,7 +1631,8 @@ export default class SourceFileLinter {
16231631

16241632
// Get the NodeType in order to check whether this is indirect global access via Window
16251633
const nodeType = this.checker.getTypeAtLocation(exprNode);
1626-
if (isGlobalThis(this.checker.typeToString(nodeType))) {
1634+
const isGlobalThisAccess = isGlobalThis(this.checker.typeToString(nodeType));
1635+
if (isGlobalThisAccess) {
16271636
// In case of Indirect global access we need to check for
16281637
// a global UI5 variable on the right side of the expression instead of left
16291638
if (ts.isPropertyAccessExpression(node)) {
@@ -1652,6 +1661,33 @@ export default class SourceFileLinter {
16521661
node,
16531662
fix: this.getGlobalFix(node),
16541663
});
1664+
} else if (this.#projectNamespaceFirstSegment &&
1665+
exprNode.text === this.#projectNamespaceFirstSegment) {
1666+
const fullNamespace = extractNamespace(node as ts.PropertyAccessExpression);
1667+
if (fullNamespace.startsWith(this.#projectNamespaceDots + ".")) {
1668+
if (!symbol || this.#isProjectGlobalNotLocal(symbol)) {
1669+
this.#reporter.addMessage(MESSAGE.NO_PROJECT_GLOBALS, {
1670+
variableName: exprNode.text,
1671+
namespace: fullNamespace,
1672+
}, {node});
1673+
}
1674+
}
1675+
} else if (isGlobalThisAccess && this.#projectNamespaceFirstSegment &&
1676+
ts.isPropertyAccessExpression(node) && ts.isIdentifier(node.name) &&
1677+
node.name.text === this.#projectNamespaceFirstSegment &&
1678+
(!symbol || this.#isProjectGlobalNotLocal(symbol))) {
1679+
// Indirect global access via globalThis/window: e.g. window.com.example.app.utils.Helper
1680+
// node is "window.com" here — walk up to find the full PropertyAccessExpression chain
1681+
let topNode: ts.Node = node;
1682+
while (ts.isPropertyAccessExpression(topNode.parent) &&
1683+
topNode.parent.expression === topNode) {
1684+
topNode = topNode.parent;
1685+
}
1686+
const fullChain = extractNamespace(topNode as ts.PropertyAccessExpression);
1687+
this.#reporter.addMessage(MESSAGE.NO_PROJECT_GLOBALS, {
1688+
variableName: node.name.text,
1689+
namespace: fullChain,
1690+
}, {node: topNode});
16551691
}
16561692
}
16571693
}
@@ -1799,6 +1835,23 @@ export default class SourceFileLinter {
17991835
declarations.some((declaration) => checkFunction(declaration.getSourceFile()));
18001836
}
18011837

1838+
// TypeScript creates implicit Identifier-kind declarations for undeclared globals (e.g. "com").
1839+
// To avoid false positives when a local variable shadows the namespace first segment
1840+
// (e.g. "const com = {}"), check whether the symbol has an explicit user-code declaration.
1841+
#isProjectGlobalNotLocal(symbol: ts.Symbol): boolean {
1842+
const declarations = symbol.getDeclarations();
1843+
if (!declarations || declarations.length === 0) return true;
1844+
for (const decl of declarations) {
1845+
const sf = decl.getSourceFile();
1846+
if (!isSourceFileOfUi5Type(sf) && !isSourceFileOfTypeScriptLib(sf) &&
1847+
(ts.isVariableDeclaration(decl) || ts.isFunctionDeclaration(decl) ||
1848+
ts.isParameter(decl) || ts.isClassDeclaration(decl))) {
1849+
return false;
1850+
}
1851+
}
1852+
return true;
1853+
}
1854+
18021855
analyzeTestsuiteThemeProperty(node: ts.PropertyAssignment) {
18031856
// Check if the node is part of a testsuite config file by its file name.
18041857
if (!VALID_TESTSUITE.test(node.getSourceFile().fileName)) return;
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
sap.ui.define([], function() {
2+
"use strict";
3+
4+
// ERROR: App-namespace global access
5+
com.example.app.utils.Formatter.format();
6+
var x = com.example.app.model.Config;
7+
com.example.app.utils.DataService = {};
8+
9+
// ERROR: Indirect global access via window
10+
window.com.example.app.utils.Helper.doSomething();
11+
var y = window.com.example.app.model.Settings;
12+
window.com.example.app.utils.Registry = {};
13+
14+
return com.example.app.utils.TestScripts;
15+
});
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
sap.ui.define(["com/example/app/utils/Helper"], function(Helper) {
2+
"use strict";
3+
4+
// OK: String literals are not property access expressions
5+
Controller.extend("com.example.app.controller.Main", {});
6+
7+
// OK: Local variable shadows namespace first segment
8+
var com = {};
9+
com.example.app.whatever;
10+
11+
// OK: Properly imported module
12+
Helper.doSomething();
13+
});

test/lib/linter/_linterHelper.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,16 @@ export function createTestsForFixtures(fixturesPath: string, fix = false) {
121121
filePaths: testFiles,
122122
fix,
123123
});
124+
} else if (fixturesPath.includes("NoProjectGlobals")) {
125+
const dirName = path.basename(fixturesPath);
126+
testDefinition({
127+
testName: dirName,
128+
namespace: "com/example/app",
129+
fileName: dirName,
130+
fixturesPath,
131+
filePaths: testFiles,
132+
fix,
133+
});
124134
} else {
125135
for (const fileName of testFiles) {
126136
if (!fileName.endsWith(".js") &&
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import {fileURLToPath} from "node:url";
2+
import {runLintRulesTests} from "../_linterHelper.js";
3+
4+
const filePath = fileURLToPath(import.meta.url);
5+
runLintRulesTests(filePath);
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
# Snapshot report for `test/lib/linter/rules/NoProjectGlobals.ts`
2+
3+
The actual snapshot is saved in `NoProjectGlobals.ts.snap`.
4+
5+
Generated by [AVA](https://avajs.dev).
6+
7+
## General: NoProjectGlobals
8+
9+
> LintResult: NoProjectGlobals
10+
11+
[
12+
{
13+
coverageInfo: [
14+
{
15+
category: 1,
16+
column: 2,
17+
line: 12,
18+
message: 'Unable to analyze this method call because the type of identifier "doSomething" in "Helper.doSomething()" could not be determined',
19+
},
20+
],
21+
errorCount: 0,
22+
fatalErrorCount: 0,
23+
filePath: 'NoProjectGlobals_Negative.js',
24+
messages: [],
25+
warningCount: 0,
26+
},
27+
{
28+
coverageInfo: [
29+
{
30+
category: 1,
31+
column: 2,
32+
line: 5,
33+
message: 'Unable to analyze this method call because the type of identifier "format" in "com.example.app.utils.Formatter.format()" could not be determined',
34+
},
35+
{
36+
category: 1,
37+
column: 2,
38+
line: 10,
39+
message: 'Unable to analyze this method call because the type of identifier "doSomething" in "window.com.example.app.utils.Helper.doSomething()" could not be determined',
40+
},
41+
],
42+
errorCount: 7,
43+
fatalErrorCount: 0,
44+
filePath: 'NoProjectGlobals.js',
45+
messages: [
46+
{
47+
column: 2,
48+
line: 5,
49+
message: 'Access of global variable \'com\' (com.example.app.utils.Formatter.format)',
50+
messageDetails: 'Do not use global variables to access UI5 projects. It might be necessary to modify the provider and consumer of this global variable. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
51+
ruleId: 'no-project-globals',
52+
severity: 2,
53+
ui5TypeInfo: undefined,
54+
},
55+
{
56+
column: 10,
57+
line: 6,
58+
message: 'Access of global variable \'com\' (com.example.app.model.Config)',
59+
messageDetails: 'Do not use global variables to access UI5 projects. It might be necessary to modify the provider and consumer of this global variable. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
60+
ruleId: 'no-project-globals',
61+
severity: 2,
62+
ui5TypeInfo: undefined,
63+
},
64+
{
65+
column: 2,
66+
line: 7,
67+
message: 'Access of global variable \'com\' (com.example.app.utils.DataService)',
68+
messageDetails: 'Do not use global variables to access UI5 projects. It might be necessary to modify the provider and consumer of this global variable. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
69+
ruleId: 'no-project-globals',
70+
severity: 2,
71+
ui5TypeInfo: undefined,
72+
},
73+
{
74+
column: 2,
75+
line: 10,
76+
message: 'Access of global variable \'com\' (window.com.example.app.utils.Helper.doSomething)',
77+
messageDetails: 'Do not use global variables to access UI5 projects. It might be necessary to modify the provider and consumer of this global variable. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
78+
ruleId: 'no-project-globals',
79+
severity: 2,
80+
ui5TypeInfo: undefined,
81+
},
82+
{
83+
column: 10,
84+
line: 11,
85+
message: 'Access of global variable \'com\' (window.com.example.app.model.Settings)',
86+
messageDetails: 'Do not use global variables to access UI5 projects. It might be necessary to modify the provider and consumer of this global variable. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
87+
ruleId: 'no-project-globals',
88+
severity: 2,
89+
ui5TypeInfo: undefined,
90+
},
91+
{
92+
column: 2,
93+
line: 12,
94+
message: 'Access of global variable \'com\' (window.com.example.app.utils.Registry)',
95+
messageDetails: 'Do not use global variables to access UI5 projects. It might be necessary to modify the provider and consumer of this global variable. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
96+
ruleId: 'no-project-globals',
97+
severity: 2,
98+
ui5TypeInfo: undefined,
99+
},
100+
{
101+
column: 9,
102+
line: 14,
103+
message: 'Access of global variable \'com\' (com.example.app.utils.TestScripts)',
104+
messageDetails: 'Do not use global variables to access UI5 projects. It might be necessary to modify the provider and consumer of this global variable. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
105+
ruleId: 'no-project-globals',
106+
severity: 2,
107+
ui5TypeInfo: undefined,
108+
},
109+
],
110+
warningCount: 0,
111+
},
112+
]
1.35 KB
Binary file not shown.

0 commit comments

Comments
 (0)