diff --git a/src/linter/ui5Types/SourceFileLinter.ts b/src/linter/ui5Types/SourceFileLinter.ts index 53297a4de..bf8d5bf0d 100644 --- a/src/linter/ui5Types/SourceFileLinter.ts +++ b/src/linter/ui5Types/SourceFileLinter.ts @@ -13,6 +13,7 @@ import { getPropertyAssignmentsInObjectLiteralExpression, findClassMember, isClassMethod, + findAmbientModuleByDeclarationName, } from "./utils.js"; import {taskStart} from "../../utils/perf.js"; import {getPositionsForNode} from "../../utils/nodePosition.js"; @@ -1439,11 +1440,13 @@ export default class SourceFileLinter { } const extractVarName = (node: ts.PropertyAccessExpression | ts.ElementAccessExpression) => { - const nodeName = ts.isPropertyAccessExpression(node) ? - node.name.text : - node.argumentExpression.getText(); - - return nodeName.replaceAll("\"", ""); + if (ts.isPropertyAccessExpression(node)) { + return node.name.text; + } else if (ts.isStringLiteralLike(node.argumentExpression)) { + return node.argumentExpression.text; + } else { + return node.argumentExpression.getText(); + } }; let exprNode = node.expression; @@ -1453,13 +1456,35 @@ export default class SourceFileLinter { namespace.unshift(extractVarName(exprNode)); exprNode = exprNode.expression; } - const exprType = this.checker.getTypeAtLocation(exprNode); - const potentialLibImport = exprType.symbol?.getName().replaceAll("\"", "") ?? ""; + const exprTypeSymbol = this.checker.getTypeAtLocation(exprNode)?.symbol; + let potentialLibImport = ""; + if (exprTypeSymbol?.valueDeclaration && ts.isModuleDeclaration(exprTypeSymbol.valueDeclaration)) { + potentialLibImport = exprTypeSymbol.valueDeclaration.name.text; + } // Checks if the left hand side is a library import. // It's sufficient just to check for "/library" at the end of the string by convention if (!potentialLibImport.endsWith("/library")) { - return; + // Fallback in case of relative imports within framework libraries, where the type does not have a symbol + const exprSymbol = this.checker.getSymbolAtLocation(exprNode); + const exprDeclaration = exprSymbol?.declarations?.[0]; + if (!exprDeclaration) { + return; + } + if (!ts.isImportDeclaration(exprDeclaration.parent)) { + return; + } + const moduleSpecifier = exprDeclaration.parent.moduleSpecifier; + const moduleSpecifierSymbol = this.checker.getSymbolAtLocation(moduleSpecifier); + const moduleName = moduleSpecifierSymbol?.valueDeclaration?.getSourceFile().fileName; + if ( + !moduleName?.startsWith("/resources/") || + !(moduleName?.endsWith("/library.js") || moduleName?.endsWith("/library.ts")) + ) { + return; + } + // Cut off "/resources/" (11 chars) and the ".(js|ts)" extension (3 chars) + potentialLibImport = moduleName.slice(11, -3); } const varName = extractVarName(node); @@ -1471,10 +1496,8 @@ export default class SourceFileLinter { // Check if the module is registered within ambient modules const ambientModules = this.checker.getAmbientModules(); - const libAmbientModule = ambientModules.find((module) => - module.name === `"${potentialLibImport}"`); - const isRegisteredAsUi5Module = ambientModules.some((module) => - module.name === `"${moduleName}"`); + const libAmbientModule = findAmbientModuleByDeclarationName(ambientModules, potentialLibImport); + const isRegisteredAsUi5Module = !!findAmbientModuleByDeclarationName(ambientModules, moduleName); let isModuleExported = true; let libExports = libAmbientModule?.exports ?? new Map(); diff --git a/src/linter/ui5Types/utils.ts b/src/linter/ui5Types/utils.ts index 18f8ec4ea..11b854ff8 100644 --- a/src/linter/ui5Types/utils.ts +++ b/src/linter/ui5Types/utils.ts @@ -176,3 +176,14 @@ export function getPropertyAssignmentsInObjectLiteralExpression( } return properties; } + +export function findAmbientModuleByDeclarationName( + ambientModules: ts.Symbol[], moduleName: string +): ts.Symbol | undefined { + return ambientModules.find((symbol) => { + if (!symbol.valueDeclaration || !ts.isModuleDeclaration(symbol.valueDeclaration)) { + return false; + } + return symbol.valueDeclaration.name.text === moduleName; + }); +} diff --git a/test/fixtures/linter/projects/sap.ui.unified/src/sap/ui/unified/ColorPicker.js b/test/fixtures/linter/projects/sap.ui.unified/src/sap/ui/unified/ColorPicker.js new file mode 100644 index 000000000..9b071b94d --- /dev/null +++ b/test/fixtures/linter/projects/sap.ui.unified/src/sap/ui/unified/ColorPicker.js @@ -0,0 +1,13 @@ +sap.ui.define([ + // This file covers the case where a framework library contains a relative import + // to a library module from which an implicit global is accessed. + "./library" +], function (Library) { + "use strict"; + + var ColorPickerMode = Library['ColorPickerMode'], + + // This is a special case as ColorPickerDisplayMode is defined in its own module but also exported via the library module. + // However, this export from the library module is not reflected in the types / api.json, so it is still reported as implicit global usage. + ColorPickerDisplayMode = Library['ColorPickerDisplayMode']; +}); diff --git a/test/fixtures/linter/projects/sap.ui.unified/src/sap/ui/unified/library.js b/test/fixtures/linter/projects/sap.ui.unified/src/sap/ui/unified/library.js new file mode 100644 index 000000000..c5222d097 --- /dev/null +++ b/test/fixtures/linter/projects/sap.ui.unified/src/sap/ui/unified/library.js @@ -0,0 +1,45 @@ +/*! + * ${copyright} + */ + +/** + * Initialization Code and shared classes of library sap.ui.unified. + */ +sap.ui.define( + [ + "sap/ui/core/Lib", + "sap/ui/unified/ColorPickerDisplayMode", + ], + function ( + Library, + ColorPickerDisplayMode, + ) { + "use strict"; + + /** + * Unified controls intended for both, mobile and desktop scenarios + * + * @namespace + * @alias sap.ui.unified + * @author SAP SE + * @version ${version} + * @since 1.28 + * @public + */ + var thisLib = Library.init({ + name: "sap.ui.unified", + apiVersion: 2, + version: "${version}", + dependencies: ["sap.ui.core"], + designtime: "sap/ui/unified/designtime/library.designtime", + types: [ + "sap.ui.unified.ColorPickerDisplayMode", + ], + }); + + // expose imported enum as property of library namespace, for documentation see ColorPickerDisplayMode.js + thisLib.ColorPickerDisplayMode = ColorPickerDisplayMode; + + return thisLib; + } +); diff --git a/test/fixtures/linter/rules/NoGlobals/NoExportedLibValues.js b/test/fixtures/linter/rules/NoGlobals/NoExportedLibValues.js index 5df779f97..f77219f99 100644 --- a/test/fixtures/linter/rules/NoGlobals/NoExportedLibValues.js +++ b/test/fixtures/linter/rules/NoGlobals/NoExportedLibValues.js @@ -5,10 +5,11 @@ sap.ui.define( var CalendarDayType = unifiedLibrary.CalendarDayType, DateRange = unifiedLibrary["DateRange"], + DateRange2 = unifiedLibrary['DateRange'], DateTypeRange = unifiedLibrary.DateTypeRange, DOMAttribute = coreLibrary.tmpl.DOMAttribute, DOMAttribute2 = coreLibrary["tmpl"].DOMAttribute, - DOMAttribute3 = coreLibrary["tmpl"]["DOMAttribute"], - DOMAttribute4 = coreLibrary.tmpl["DOMAttribute"]; + DOMAttribute3 = coreLibrary['tmpl']["DOMAttribute"], + DOMAttribute4 = coreLibrary.tmpl['DOMAttribute']; } ); diff --git a/test/lib/linter/linter.ts b/test/lib/linter/linter.ts index 2a0e6c7c7..1f40cddde 100644 --- a/test/lib/linter/linter.ts +++ b/test/lib/linter/linter.ts @@ -232,6 +232,17 @@ test.serial("lint: All files of library with sap.ui.suite namespace", async (t) t.snapshot(preprocessLintResultsForSnapshot(res)); }); +test.serial("lint: All files of library with sap.ui.unified namespace", async (t) => { + const projectPath = path.join(fixturesProjectsPath, "sap.ui.unified"); + const {lintProject} = t.context; + + const res = await lintProject({ + rootDir: projectPath, + }, t.context.sharedLanguageService); + + t.snapshot(preprocessLintResultsForSnapshot(res)); +}); + test.serial("lint: All files of com.ui5.troublesome.app with custom config", async (t) => { const projectPath = path.join(fixturesProjectsPath, "com.ui5.troublesome.app"); const {lintProject} = t.context; diff --git a/test/lib/linter/rules/snapshots/NoGlobals.ts.md b/test/lib/linter/rules/snapshots/NoGlobals.ts.md index c12cef9f2..0866fa64e 100644 --- a/test/lib/linter/rules/snapshots/NoGlobals.ts.md +++ b/test/lib/linter/rules/snapshots/NoGlobals.ts.md @@ -388,7 +388,7 @@ Generated by [AVA](https://avajs.dev). [ { coverageInfo: [], - errorCount: 6, + errorCount: 7, fatalErrorCount: 0, filePath: 'NoExportedLibValues.js', messages: [ @@ -401,8 +401,16 @@ Generated by [AVA](https://avajs.dev). severity: 2, }, { - column: 35, + column: 32, line: 8, + message: 'Access of module \'sap/ui/unified/DateRange\' (unifiedLibrary.DateRange) not exported by library \'sap/ui/unified/library\'', + messageDetails: 'Please import the module itself directly instead of accessing it via the library module.', + ruleId: 'no-implicit-globals', + severity: 2, + }, + { + column: 35, + line: 9, message: 'Access of module \'sap/ui/unified/DateTypeRange\' (unifiedLibrary.DateTypeRange) not exported by library \'sap/ui/unified/library\'', messageDetails: 'Please import the module itself directly instead of accessing it via the library module.', ruleId: 'no-implicit-globals', @@ -410,7 +418,7 @@ Generated by [AVA](https://avajs.dev). }, { column: 36, - line: 9, + line: 10, message: 'Access of module \'sap/ui/core/tmpl/DOMAttribute\' (coreLibrary.tmpl.DOMAttribute) not exported by library \'sap/ui/core/library\'', messageDetails: 'Please import the module itself directly instead of accessing it via the library module.', ruleId: 'no-implicit-globals', @@ -418,7 +426,7 @@ Generated by [AVA](https://avajs.dev). }, { column: 40, - line: 10, + line: 11, message: 'Access of module \'sap/ui/core/tmpl/DOMAttribute\' (coreLibrary.tmpl.DOMAttribute) not exported by library \'sap/ui/core/library\'', messageDetails: 'Please import the module itself directly instead of accessing it via the library module.', ruleId: 'no-implicit-globals', @@ -426,7 +434,7 @@ Generated by [AVA](https://avajs.dev). }, { column: 40, - line: 11, + line: 12, message: 'Access of module \'sap/ui/core/tmpl/DOMAttribute\' (coreLibrary.tmpl.DOMAttribute) not exported by library \'sap/ui/core/library\'', messageDetails: 'Please import the module itself directly instead of accessing it via the library module.', ruleId: 'no-implicit-globals', @@ -434,7 +442,7 @@ Generated by [AVA](https://avajs.dev). }, { column: 37, - line: 12, + line: 13, message: 'Access of module \'sap/ui/core/tmpl/DOMAttribute\' (coreLibrary.tmpl.DOMAttribute) not exported by library \'sap/ui/core/library\'', messageDetails: 'Please import the module itself directly instead of accessing it via the library module.', ruleId: 'no-implicit-globals', diff --git a/test/lib/linter/rules/snapshots/NoGlobals.ts.snap b/test/lib/linter/rules/snapshots/NoGlobals.ts.snap index d175b88fa..04cc616b6 100644 Binary files a/test/lib/linter/rules/snapshots/NoGlobals.ts.snap and b/test/lib/linter/rules/snapshots/NoGlobals.ts.snap differ diff --git a/test/lib/linter/snapshots/linter.ts.md b/test/lib/linter/snapshots/linter.ts.md index 06b5f4cf6..b5c7c2314 100644 --- a/test/lib/linter/snapshots/linter.ts.md +++ b/test/lib/linter/snapshots/linter.ts.md @@ -3055,6 +3055,29 @@ Generated by [AVA](https://avajs.dev). }, ] +## lint: All files of library with sap.ui.unified namespace + +> Snapshot 1 + + [ + { + coverageInfo: [], + errorCount: 1, + fatalErrorCount: 0, + filePath: 'src/sap/ui/unified/ColorPicker.js', + messages: [ + { + column: 36, + line: 12, + message: 'Access of module \'sap/ui/unified/ColorPickerDisplayMode\' (Library.ColorPickerDisplayMode) not exported by library \'sap/ui/unified/library\'', + ruleId: 'no-implicit-globals', + severity: 2, + }, + ], + warningCount: 0, + }, + ] + ## lint: All files of com.ui5.troublesome.app with custom config > Snapshot 1 diff --git a/test/lib/linter/snapshots/linter.ts.snap b/test/lib/linter/snapshots/linter.ts.snap index 568ce35e8..524f127e1 100644 Binary files a/test/lib/linter/snapshots/linter.ts.snap and b/test/lib/linter/snapshots/linter.ts.snap differ