Skip to content

Commit

Permalink
fix: False-positive global findings in controllers
Browse files Browse the repository at this point in the history 
This solves an issue where the linter would falsely report access to
global variables in controller code when accessing the controller class
(e.g. by adding methods to the prototype).

The issue was caused by the introduction of type support for "byId" in
controllers and only appears when there is a view that references the
controller.

Caused by: #423
  • Loading branch information
@matz3
matz3 committed Nov 28, 2024
1 parent b4c326f commit b48a2dc
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 11 deletions.
4 changes: 2 additions & 2 deletions src/linter/ui5Types/SourceFileLinter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ interface DeprecationInfo {
}

function isSourceFileOfUi5Type(sourceFile: ts.SourceFile) {
return /@openui5|@sapui5|@ui5/.test(sourceFile.fileName);
return /\/types\/(@openui5|@sapui5|@ui5\/linter\/overrides)\//.test(sourceFile.fileName);
}

function isSourceFileOfUi5OrThirdPartyType(sourceFile: ts.SourceFile) {
return /@openui5|@sapui5|@ui5|@types\/jquery/.test(sourceFile.fileName);
return isSourceFileOfUi5Type(sourceFile) || /\/types\/(@types\/jquery)\//.test(sourceFile.fileName);
}

function isSourceFileOfJquerySapType(sourceFile: ts.SourceFile) {
Expand Down
14 changes: 13 additions & 1 deletion test/fixtures/linter/projects/com.ui5.troublesome.app/webapp/controller/Main.controller.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
sap.ui.define(["./BaseController", "sap/m/MessageBox"], function (BaseController, MessageBox) {
"use strict";

return BaseController.extend("com.ui5.troublesome.app.controller.Main", {
const MainController = BaseController.extend("com.ui5.troublesome.app.controller.Main", {
sayHello: function () {
MessageBox.show("Hello World!");
},
Expand Down Expand Up @@ -32,4 +32,16 @@ sap.ui.define(["./BaseController", "sap/m/MessageBox"], function (BaseController
this.getView().byId("unknown").prop("foo", "bar");
}
});

// Note: Accessing the controller prototype should not cause a false positive for global variables,
// which was appearing in combination with the declaration merging for the byId type support.
MainController.prototype.doSomething = function () {

// byId type support should also work within methods attached to the prototype
this.byId("helloButton").attachTap(function() {
console.log("Tapped");
});
};

return MainController;
});
29 changes: 26 additions & 3 deletions test/lib/linter/snapshots/linter.ts.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ Generated by [AVA](https://avajs.dev).
},
{
coverageInfo: [],
errorCount: 6,
errorCount: 7,
fatalErrorCount: 0,
filePath: 'webapp/controller/Main.controller.js',
messages: [
Expand Down Expand Up @@ -467,6 +467,14 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 28,
line: 41,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-api',
severity: 2,
},
],
warningCount: 0,
},
Expand Down Expand Up @@ -1147,7 +1155,7 @@ Generated by [AVA](https://avajs.dev).
},
{
coverageInfo: [],
errorCount: 6,
errorCount: 7,
fatalErrorCount: 0,
filePath: 'webapp/controller/Main.controller.js',
messages: [
Expand Down Expand Up @@ -1193,6 +1201,13 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 28,
line: 41,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
ruleId: 'no-deprecated-api',
severity: 2,
},
],
warningCount: 0,
},
Expand Down Expand Up @@ -2623,7 +2638,7 @@ Generated by [AVA](https://avajs.dev).
},
{
coverageInfo: [],
errorCount: 6,
errorCount: 7,
fatalErrorCount: 0,
filePath: 'webapp/controller/Main.controller.js',
messages: [
Expand Down Expand Up @@ -2675,6 +2690,14 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 28,
line: 41,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-api',
severity: 2,
},
],
warningCount: 0,
},
Expand Down
Binary file modified test/lib/linter/snapshots/linter.ts.snap
View file
Binary file not shown.
45 changes: 40 additions & 5 deletions test/lib/snapshots/index.ts.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ Generated by [AVA](https://avajs.dev).
},
{
coverageInfo: [],
errorCount: 6,
errorCount: 7,
fatalErrorCount: 0,
filePath: 'webapp/controller/Main.controller.js',
messages: [
Expand Down Expand Up @@ -103,6 +103,13 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 28,
line: 41,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
ruleId: 'no-deprecated-api',
severity: 2,
},
],
warningCount: 0,
},
Expand Down Expand Up @@ -454,7 +461,7 @@ Generated by [AVA](https://avajs.dev).
},
{
coverageInfo: [],
errorCount: 6,
errorCount: 7,
fatalErrorCount: 0,
filePath: 'webapp/controller/Main.controller.js',
messages: [
Expand Down Expand Up @@ -500,6 +507,13 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 28,
line: 41,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
ruleId: 'no-deprecated-api',
severity: 2,
},
],
warningCount: 0,
},
Expand Down Expand Up @@ -1455,7 +1469,7 @@ Generated by [AVA](https://avajs.dev).
},
{
coverageInfo: [],
errorCount: 6,
errorCount: 7,
fatalErrorCount: 0,
filePath: 'webapp/controller/Main.controller.js',
messages: [
Expand Down Expand Up @@ -1501,6 +1515,13 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 28,
line: 41,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
ruleId: 'no-deprecated-api',
severity: 2,
},
],
warningCount: 0,
},
Expand Down Expand Up @@ -1852,7 +1873,7 @@ Generated by [AVA](https://avajs.dev).
},
{
coverageInfo: [],
errorCount: 6,
errorCount: 7,
fatalErrorCount: 0,
filePath: 'webapp/controller/Main.controller.js',
messages: [
Expand Down Expand Up @@ -1898,6 +1919,13 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 28,
line: 41,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
ruleId: 'no-deprecated-api',
severity: 2,
},
],
warningCount: 0,
},
Expand Down Expand Up @@ -2249,7 +2277,7 @@ Generated by [AVA](https://avajs.dev).
},
{
coverageInfo: [],
errorCount: 6,
errorCount: 7,
fatalErrorCount: 0,
filePath: 'webapp/controller/Main.controller.js',
messages: [
Expand Down Expand Up @@ -2295,6 +2323,13 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 28,
line: 41,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
ruleId: 'no-deprecated-api',
severity: 2,
},
],
warningCount: 0,
},
Expand Down
Binary file modified test/lib/snapshots/index.ts.snap
View file
Binary file not shown.

0 comments on commit b48a2dc

Please sign in to comment.