diff --git a/src/linter/ui5Types/SourceFileLinter.ts b/src/linter/ui5Types/SourceFileLinter.ts index ded0a3512..3438eda1e 100644 --- a/src/linter/ui5Types/SourceFileLinter.ts +++ b/src/linter/ui5Types/SourceFileLinter.ts @@ -1,4 +1,4 @@ -import ts, {SymbolFlags} from "typescript"; +import ts from "typescript"; import path from "node:path/posix"; import {getLogger} from "@ui5/logger"; import SourceFileReporter from "./SourceFileReporter.js"; @@ -282,54 +282,7 @@ export default class SourceFileLinter { !decl.getSourceFile().isDeclarationFile); // If the original raw render file is available, we can analyze it directly - if (originalDeclarations && originalDeclarations.length > 0) { - originalDeclarations.forEach((declaration) => this.analyzeControlRendererInternals(declaration)); - } else { - // When there's an ambient module, then we need special handling. - // In case we have the raw file & typescript definitions for it, their types - // are being merged, creating an ambient module. We need to find the original - // raw file in order to analyze the renderer. Such special case, is for example - // any openui5 library when we analyze the openui5 repo- we have the TS definitions - // loaded in the TS compiler via @sapui5/types, but we actually want to analyze the - // source files of those types. - let importModuleString = ""; - - // Get the import string module from the current source file, so we can filter later - const rendererSymbol = this.checker.getSymbolAtLocation(rendererMember.initializer); - const importedRenderModule = rendererSymbol?.getDeclarations()?.[0]?.parent; - - if (importedRenderModule && - ts.isImportDeclaration(importedRenderModule) && - ts.isStringLiteral(importedRenderModule.moduleSpecifier)) { - importModuleString = importedRenderModule.moduleSpecifier.text; - } - - // Find the correct raw source file - const exportSourceFile = this.program.getSourceFiles().find((sourceFile) => - sourceFile.fileName.includes(importModuleString)); - - // Extract the exports from the source file - const exportFileSymbol = exportSourceFile && this.checker.getSymbolAtLocation(exportSourceFile); - const moduleExportsSymbols = exportFileSymbol && this.checker.getExportsOfModule(exportFileSymbol); - - // Check all exports - moduleExportsSymbols?.forEach((exportSymbol) => { - // Note: getAliasedSymbol fails when called with a symbol that isn't an alias - let symbol = exportSymbol; - if (exportSymbol.flags & SymbolFlags.TypeAlias) { - // Export could be a "default", so we need to get the real reference of the export symbol - symbol = this.checker.getAliasedSymbol(exportSymbol); - } - - symbol?.getDeclarations()?.forEach((declaration) => { - if (ts.isVariableDeclaration(declaration) && declaration.initializer) { - this.analyzeControlRendererInternals(declaration.initializer); - } else if (ts.isExportAssignment(declaration) && declaration.expression) { - this.analyzeControlRendererInternals(declaration.expression); - } - }); - }); - } + originalDeclarations?.forEach((declaration) => this.analyzeControlRendererInternals(declaration)); } else { // Analyze renderer property when it's directly embedded in the renderer object // i.e. { renderer: {apiVersion: "2", render: () => {}} } diff --git a/src/linter/ui5Types/TypeLinter.ts b/src/linter/ui5Types/TypeLinter.ts index 5250e493c..e4b540db5 100644 --- a/src/linter/ui5Types/TypeLinter.ts +++ b/src/linter/ui5Types/TypeLinter.ts @@ -59,7 +59,12 @@ export default class TypeChecker { if (namespace) { // Map namespace used in imports (without /resources) to /resources paths this.#compilerOptions.paths = { - [`${namespace}/*`]: [`/resources/${namespace}/*`], + [`${namespace}/*`]: [ + // Enforce that the compiler also tries to resolve imports with a .js extension. + // With this mapping, the compiler still first tries to resolve the .ts, .tsx and .d.ts extensions + // but then falls back to .js + `/resources/${namespace}/*.js`, + ], }; } } diff --git a/src/linter/ui5Types/host.ts b/src/linter/ui5Types/host.ts index ffeffeb75..a469f9073 100644 --- a/src/linter/ui5Types/host.ts +++ b/src/linter/ui5Types/host.ts @@ -47,17 +47,26 @@ async function collectSapui5TypesFiles() { return typesFiles; } -function addSapui5TypesMappingToCompilerOptions(sapui5TypesFiles: string[], options: ts.CompilerOptions) { +function addSapui5TypesMappingToCompilerOptions( + sapui5TypesFiles: string[], options: ts.CompilerOptions, projectNamespace?: string +) { const paths = options.paths ?? (options.paths = {}); sapui5TypesFiles.forEach((fileName) => { - if (fileName === "sap.ui.core.d.ts") { - // No need to add a mapping for sap.ui.core, as it is loaded by default - return; - } + const dtsPath = `/types/@sapui5/types/types/${fileName}`; const libraryName = posixPath.basename(fileName, ".d.ts"); - const namespace = libraryName.replace(/\./g, "/") + "/*"; - const pathsEntry = paths[namespace] ?? (paths[namespace] = []); - pathsEntry.push(`/types/@sapui5/types/types/${fileName}`); + const libraryNamespace = libraryName.replace(/\./g, "/"); + if (libraryNamespace === "sap/ui/core" || libraryNamespace === projectNamespace) { + // Special cases: + // - sap.ui.core needs to be loaded by default as it provides general API that must always be available + // - When linting a framework library, the corresponding types should be loaded by default as they + // might not get loaded by the compiler otherwise, as the actual sources are available in the project. + options.types?.push(dtsPath); + } else { + // For other framework libraries we can add a paths mapping to load them on demand + const mappingNamespace = libraryNamespace + "/*"; + const pathsEntry = paths[mappingNamespace] ?? (paths[mappingNamespace] = []); + pathsEntry.push(dtsPath); + } }); } @@ -70,6 +79,9 @@ export async function createVirtualCompilerHost( ): Promise { const silly = log.isLevelEnabled("silly"); + options.typeRoots = ["/types"]; + options.types = []; + const typePathMappings = new Map(); addPathMappingForPackage("typescript", typePathMappings); @@ -85,16 +97,11 @@ export async function createVirtualCompilerHost( require.resolve("../../../resources/overrides/package.json") )); - options.typeRoots = ["/types"]; - options.types = [ - // Request compiler to only use sap.ui.core types by default - other types will be loaded on demand - // (see addSapui5TypesMappingToCompilerOptions) - ...typePackageDirs.filter((dir) => dir !== "/types/@sapui5/types/"), - "/types/@sapui5/types/types/sap.ui.core.d.ts", - ]; + // Add all types except @sapui5/types which will be handled below + options.types.push(...typePackageDirs.filter((dir) => dir !== "/types/@sapui5/types/")); - // Adds mappings for all other sapui5 types, so that they are only loaded once a module is imported - addSapui5TypesMappingToCompilerOptions(await collectSapui5TypesFiles(), options); + // Adds types / mappings for all @sapui5/types + addSapui5TypesMappingToCompilerOptions(await collectSapui5TypesFiles(), options, context.getNamespace()); // Create regex matching all path mapping keys const pathMappingRegex = new RegExp( diff --git a/test/fixtures/linter/projects/sap.f/src/sap/f/NewControl.js b/test/fixtures/linter/projects/sap.f/src/sap/f/NewControl.js new file mode 100644 index 000000000..efe886c0f --- /dev/null +++ b/test/fixtures/linter/projects/sap.f/src/sap/f/NewControl.js @@ -0,0 +1,12 @@ +sap.ui.define([ + "sap/ui/core/Control" +], function (Control) { + "use strict"; + // This control is used to test cases where a framework library contains a control + // for which no @sapui5/types exist. + // The control and usages of it should still be analyzed properly, e.g. when extending it + // and not defining a renderer. + return Control.extend("sap.f.NewControl", { + renderer: null + }); +}); diff --git a/test/fixtures/linter/projects/sap.f/test/sap/f/LinterTest.js b/test/fixtures/linter/projects/sap.f/test/sap/f/LinterTest.js index 40c6f6fb0..e802ec46a 100644 --- a/test/fixtures/linter/projects/sap.f/test/sap/f/LinterTest.js +++ b/test/fixtures/linter/projects/sap.f/test/sap/f/LinterTest.js @@ -3,15 +3,18 @@ sap.ui.require([ "sap/f/Avatar", "sap/m/DateTimeInput", - "sap/f/ProductSwitchRenderer" -], (Avatar, DateTimeInput, ProductSwitchRenderer) => { + "sap/f/ProductSwitchRenderer", + "sap/f/NewControl" +], (Avatar, DateTimeInput, ProductSwitchRenderer, NewControl) => { new Avatar(); new DateTimeInput(); Avatar.extend("CustomControl", { // Usage of render function without object is deprecated as no apiVersion is defined - // TODO detect: This is currently not detected as the module resolution is not working correctly. - // This needs to be solved in a separate change. renderer: ProductSwitchRenderer.render }); + + // This ensures that the renderer declaration is also checked for controls based on + // controls of a framework library for which no @sapui5/types exist (sap/f/NewControl). + const CustomNewControl = NewControl.extend("CustomNewControl"); }); diff --git a/test/lib/linter/snapshots/linter.ts.md b/test/lib/linter/snapshots/linter.ts.md index b0b54391b..8fae968a7 100644 --- a/test/lib/linter/snapshots/linter.ts.md +++ b/test/lib/linter/snapshots/linter.ts.md @@ -2085,6 +2085,14 @@ Generated by [AVA](https://avajs.dev). messages: [], warningCount: 0, }, + { + coverageInfo: [], + errorCount: 0, + fatalErrorCount: 0, + filePath: 'src/sap/f/NewControl.js', + messages: [], + warningCount: 0, + }, { coverageInfo: [], errorCount: 0, @@ -2127,10 +2135,18 @@ Generated by [AVA](https://avajs.dev). }, { coverageInfo: [], - errorCount: 2, + errorCount: 4, fatalErrorCount: 0, filePath: 'test/sap/f/LinterTest.js', messages: [ + { + column: 13, + line: 14, + message: 'Use of deprecated renderer detected. Define explicitly the {apiVersion: 2} parameter in the renderer object', + messageDetails: '"Renderer Object (https://ui5.sap.com/#/topic/c9ab34570cc14ea5ab72a6d1a4a03e3f)",', + ruleId: 'no-deprecated-api', + severity: 2, + }, { column: 2, line: 4, @@ -2147,6 +2163,14 @@ Generated by [AVA](https://avajs.dev). ruleId: 'no-deprecated-api', severity: 2, }, + { + column: 27, + line: 19, + message: 'Control \'CustomNewControl\' is missing a renderer declaration', + messageDetails: 'Not defining a \'renderer\' for control \'CustomNewControl\' may lead to synchronous loading of the \'CustomNewControlRenderer\' module. If no renderer exists, set \'renderer: null\'. Otherwise, either import the renderer module and assign it to the \'renderer\' property or implement the renderer inline.', + ruleId: 'no-deprecated-control-renderer-declaration', + severity: 2, + }, ], warningCount: 0, }, diff --git a/test/lib/linter/snapshots/linter.ts.snap b/test/lib/linter/snapshots/linter.ts.snap index 566628c28..0c5f21383 100644 Binary files a/test/lib/linter/snapshots/linter.ts.snap and b/test/lib/linter/snapshots/linter.ts.snap differ