Skip to content

Commit

Permalink
fix: Detect more deprecated renderer declarations
Browse files Browse the repository at this point in the history
When a function is used, it can not have an apiVersion defined, so the
declaration is deprecated and should be detected as such.
  • Loading branch information
matz3 committed Dec 6, 2024
1 parent f416a0c commit 74f65bf
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 4 deletions.
11 changes: 10 additions & 1 deletion src/linter/ui5Types/SourceFileLinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,9 @@ export default class SourceFileLinter {
return apiVersionNode;
};

const nodeType = this.checker.getTypeAtLocation(node);
const nodeValueDeclaration = nodeType.getSymbol()?.valueDeclaration;

// Analyze renderer property when it's an ObjectLiteralExpression
// i.e. { renderer: {apiVersion: "2", render: () => {}} }
if (node && (ts.isObjectLiteralExpression(node) || ts.isVariableDeclaration(node))) {
Expand Down Expand Up @@ -420,7 +423,13 @@ export default class SourceFileLinter {
this.analyzeIconCallInRenderMethod(node);
// Analyze renderer property when it's a function i.e. { renderer: () => {} }
} else if (ts.isMethodDeclaration(node) || ts.isArrowFunction(node) ||
ts.isFunctionExpression(node) || ts.isFunctionDeclaration(node)) {
ts.isFunctionExpression(node) || ts.isFunctionDeclaration(node) || (
nodeValueDeclaration && (
ts.isFunctionExpression(nodeValueDeclaration) ||
ts.isFunctionDeclaration(nodeValueDeclaration) ||
ts.isArrowFunction(nodeValueDeclaration)
)
)) {
// reporter.addMessage() won't work in this case as it's bound to the current analyzed file.
// The findings can be in different file i.e. Control being analyzed,
// but reporting might be in ControlRenderer
Expand Down
12 changes: 10 additions & 2 deletions test/fixtures/linter/projects/sap.f/test/sap/f/LinterTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,16 @@

sap.ui.require([
"sap/f/Avatar",
"sap/m/DateTimeInput"
], (Avatar, DateTimeInput) => {
"sap/m/DateTimeInput",
"sap/f/ProductSwitchRenderer"
], (Avatar, DateTimeInput, ProductSwitchRenderer) => {
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
});
});
8 changes: 8 additions & 0 deletions test/fixtures/linter/rules/renderer/10Control.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
sap.ui.define(["sap/ui/core/Control", "./8ControlRenderer"], function (Control, Renderer) {
var myControl = Control.extend("mycomp.myControl", {
metadata: {},
renderer: Renderer.render,
});

return myControl;
});
11 changes: 11 additions & 0 deletions test/fixtures/linter/rules/renderer/11Control.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
sap.ui.define(["sap/ui/core/Control"], function (Control) {
function render(oRm, oMyControl) {}
const Renderer = {render};

var myControl = Control.extend("mycomp.myControl", {
metadata: {},
renderer: Renderer.render,
});

return myControl;
});
2 changes: 1 addition & 1 deletion test/fixtures/linter/rules/renderer/2ControlRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ sap.ui.define([], function () {

myControlRenderer.apiVersion = 1; // Deprecated

myControlRenderer.render = function (oRm, oMyControl) {};
myControlRenderer.render = (oRm, oMyControl) => {};

return myControlRenderer;
});
8 changes: 8 additions & 0 deletions test/fixtures/linter/rules/renderer/9Control.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
sap.ui.define(["sap/ui/core/Control", "./2ControlRenderer"], function (Control, Renderer) {
var myControl = Control.extend("mycomp.myControl", {
metadata: {},
renderer: Renderer.render,
});

return myControl;
});
51 changes: 51 additions & 0 deletions test/lib/linter/rules/snapshots/renderer.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,40 @@ Generated by [AVA](https://avajs.dev).
> Snapshot 1
[
{
coverageInfo: [],
errorCount: 1,
fatalErrorCount: 0,
filePath: '10Control.js',
messages: [
{
column: 13,
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,
},
],
warningCount: 0,
},
{
coverageInfo: [],
errorCount: 1,
fatalErrorCount: 0,
filePath: '11Control.js',
messages: [
{
column: 13,
line: 7,
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,
},
],
warningCount: 0,
},
{
coverageInfo: [
{
Expand Down Expand Up @@ -376,6 +410,23 @@ Generated by [AVA](https://avajs.dev).
],
warningCount: 0,
},
{
coverageInfo: [],
errorCount: 1,
fatalErrorCount: 0,
filePath: '9Control.js',
messages: [
{
column: 13,
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,
},
],
warningCount: 0,
},
{
coverageInfo: [],
errorCount: 3,
Expand Down
Binary file modified test/lib/linter/rules/snapshots/renderer.ts.snap
Binary file not shown.

0 comments on commit 74f65bf

Please sign in to comment.