Skip to content

Commit

Permalink
fix: Properly check directly exported renderer
Browse files Browse the repository at this point in the history
There was a gap with the detection of Control renderers that are
declared in a separate module and directly returned within the module
without using a locale variable.
This issue was only reproducible within the context of UI5 framework
libraries where type definitions via @sapui5/types are available in
addition to the actual source code.
For that reason a specific test case within the sap.f test project has
been added.
  • Loading branch information
matz3 committed Nov 15, 2024
1 parent cf6907c commit 91ddd39
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 4 deletions.
14 changes: 10 additions & 4 deletions src/linter/ui5Types/SourceFileLinter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import ts, {Identifier} from "typescript";
import ts, {Identifier, SymbolFlags} from "typescript";
import path from "node:path/posix";
import {getLogger} from "@ui5/logger";
import SourceFileReporter from "./SourceFileReporter.js";
Expand Down Expand Up @@ -313,12 +313,18 @@ export default class SourceFileLinter {

// Check all exports
moduleExportsSymbols?.forEach((exportSymbol) => {
// Export could be a "default", so we need to get the real reference of the export symbol
const exportSymbolAlias = this.checker.getAliasedSymbol(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);
}

exportSymbolAlias?.getDeclarations()?.forEach((declaration) => {
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);
}
});
});
Expand Down
13 changes: 13 additions & 0 deletions test/fixtures/linter/projects/sap.f/src/sap/f/ProductSwitch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
sap.ui.define([
"sap/ui/core/Control",

// NOTE: For this test case it is important to use the full namespace, not a relative import here.
// It makes a difference on how TypeScript resolves types and how SourceFileLinter analyses the renderer declaration
"sap/f/ProductSwitchRenderer"
], function (Control, ProductSwitchRenderer) {
"use strict";
var ProductSwitch = Control.extend("sap.f.ProductSwitch", {
renderer: ProductSwitchRenderer
});
return ProductSwitch;
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
sap.ui.define(function () {
"use strict";
return {
apiVersion: 1, // Reported: apiVersion property must be present and to have value 2
render: function (oRm, oControl) {
// Reported: IconPool is NOT declared as dependency
oRm.icon("sap-icon://appointment", null, { title: null });
}
};
});
40 changes: 40 additions & 0 deletions test/lib/linter/snapshots/linter.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -1914,6 +1914,46 @@ Generated by [AVA](https://avajs.dev).
messages: [],
warningCount: 0,
},
{
coverageInfo: [],
errorCount: 0,
fatalErrorCount: 0,
filePath: 'src/sap/f/ProductSwitch.js',
messages: [],
warningCount: 0,
},
{
coverageInfo: [
{
category: 1,
column: 4,
line: 7,
message: 'Unable to analyze this method call because the type of identifier "icon" in "oRm.icon("sap-icon://appointment", null, { title: null })"" could not be determined',
},
],
errorCount: 2,
fatalErrorCount: 0,
filePath: 'src/sap/f/ProductSwitchRenderer.js',
messages: [
{
column: 3,
line: 4,
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: 4,
line: 7,
message: '"sap/ui/core/IconPool" module must be imported when using RenderManager\'s icon() method',
messageDetails: '"RenderManager (https://ui5.sap.com/1.120/#/api/sap.ui.core.RenderManager#methods/icon)",',
ruleId: 'no-deprecated-api',
severity: 2,
},
],
warningCount: 0,
},
{
coverageInfo: [],
errorCount: 2,
Expand Down
Binary file modified test/lib/linter/snapshots/linter.ts.snap
Binary file not shown.

0 comments on commit 91ddd39

Please sign in to comment.