diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 7d3dea677..2b918b03f 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -35,7 +35,7 @@ "@eslint/js": "^9.15.0", "@istanbuljs/esm-loader-hook": "^0.3.0", "@istanbuljs/nyc-config-typescript": "^1.0.2", - "@stylistic/eslint-plugin": "^3.0.0", + "@stylistic/eslint-plugin": "^3.0.1", "@types/he": "^1.2.3", "@types/node": "20.11.0", "@types/sinon": "^17.0.3", @@ -3560,13 +3560,13 @@ "dev": true }, "node_modules/@stylistic/eslint-plugin": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/@stylistic/eslint-plugin/-/eslint-plugin-3.0.0.tgz", - "integrity": "sha512-9GJI6iBtGjOqSsyCKUvE6Vn7qDT52hbQaoq/SwxH6A1bciymZfvBfHIIrD3E7Koi2sjzOa/MNQ2XOguHtVJOyw==", + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/@stylistic/eslint-plugin/-/eslint-plugin-3.0.1.tgz", + "integrity": "sha512-rQ3tcT5N2cynofJfbjUsnL4seoewTaOVBLyUEwtNldo7iNMPo3h/GUQk+Cl3iHEWwRxjq2wuH6q0FufQrbVL1A==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/utils": "8.13.0", + "@typescript-eslint/utils": "^8.13.0", "eslint-visitor-keys": "^4.2.0", "espree": "^10.3.0", "estraverse": "^5.3.0", @@ -3579,150 +3579,6 @@ "eslint": ">=8.40.0" } }, - "node_modules/@stylistic/eslint-plugin/node_modules/@typescript-eslint/scope-manager": { - "version": "8.13.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/scope-manager/-/scope-manager-8.13.0.tgz", - "integrity": "sha512-XsGWww0odcUT0gJoBZ1DeulY1+jkaHUciUq4jKNv4cpInbvvrtDoyBH9rE/n2V29wQJPk8iCH1wipra9BhmiMA==", - "dev": true, - "license": "MIT", - "dependencies": { - "@typescript-eslint/types": "8.13.0", - "@typescript-eslint/visitor-keys": "8.13.0" - }, - "engines": { - "node": "^18.18.0 || ^20.9.0 || >=21.1.0" - }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/typescript-eslint" - } - }, - "node_modules/@stylistic/eslint-plugin/node_modules/@typescript-eslint/types": { - "version": "8.13.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/types/-/types-8.13.0.tgz", - "integrity": "sha512-4cyFErJetFLckcThRUFdReWJjVsPCqyBlJTi6IDEpc1GWCIIZRFxVppjWLIMcQhNGhdWJJRYFHpHoDWvMlDzng==", - "dev": true, - "license": "MIT", - "engines": { - "node": "^18.18.0 || ^20.9.0 || >=21.1.0" - }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/typescript-eslint" - } - }, - "node_modules/@stylistic/eslint-plugin/node_modules/@typescript-eslint/typescript-estree": { - "version": "8.13.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/typescript-estree/-/typescript-estree-8.13.0.tgz", - "integrity": "sha512-v7SCIGmVsRK2Cy/LTLGN22uea6SaUIlpBcO/gnMGT/7zPtxp90bphcGf4fyrCQl3ZtiBKqVTG32hb668oIYy1g==", - "dev": true, - "license": "BSD-2-Clause", - "dependencies": { - "@typescript-eslint/types": "8.13.0", - "@typescript-eslint/visitor-keys": "8.13.0", - "debug": "^4.3.4", - "fast-glob": "^3.3.2", - "is-glob": "^4.0.3", - "minimatch": "^9.0.4", - "semver": "^7.6.0", - "ts-api-utils": "^1.3.0" - }, - "engines": { - "node": "^18.18.0 || ^20.9.0 || >=21.1.0" - }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/typescript-eslint" - }, - "peerDependenciesMeta": { - "typescript": { - "optional": true - } - } - }, - "node_modules/@stylistic/eslint-plugin/node_modules/@typescript-eslint/utils": { - "version": "8.13.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/utils/-/utils-8.13.0.tgz", - "integrity": "sha512-A1EeYOND6Uv250nybnLZapeXpYMl8tkzYUxqmoKAWnI4sei3ihf2XdZVd+vVOmHGcp3t+P7yRrNsyyiXTvShFQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "@eslint-community/eslint-utils": "^4.4.0", - "@typescript-eslint/scope-manager": "8.13.0", - "@typescript-eslint/types": "8.13.0", - "@typescript-eslint/typescript-estree": "8.13.0" - }, - "engines": { - "node": "^18.18.0 || ^20.9.0 || >=21.1.0" - }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/typescript-eslint" - }, - "peerDependencies": { - "eslint": "^8.57.0 || ^9.0.0" - } - }, - "node_modules/@stylistic/eslint-plugin/node_modules/@typescript-eslint/visitor-keys": { - "version": "8.13.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/visitor-keys/-/visitor-keys-8.13.0.tgz", - "integrity": "sha512-7N/+lztJqH4Mrf0lb10R/CbI1EaAMMGyF5y0oJvFoAhafwgiRA7TXyd8TFn8FC8k5y2dTsYogg238qavRGNnlw==", - "dev": true, - "license": "MIT", - "dependencies": { - "@typescript-eslint/types": "8.13.0", - "eslint-visitor-keys": "^3.4.3" - }, - "engines": { - "node": "^18.18.0 || ^20.9.0 || >=21.1.0" - }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/typescript-eslint" - } - }, - "node_modules/@stylistic/eslint-plugin/node_modules/@typescript-eslint/visitor-keys/node_modules/eslint-visitor-keys": { - "version": "3.4.3", - "resolved": "https://registry.npmjs.org/eslint-visitor-keys/-/eslint-visitor-keys-3.4.3.tgz", - "integrity": "sha512-wpc+LXeiyiisxPlEkUzU6svyS1frIO3Mgxj1fdy7Pm8Ygzguax2N3Fa/D/ag1WqbOprdI+uY6wMUl8/a2G+iag==", - "dev": true, - "license": "Apache-2.0", - "engines": { - "node": "^12.22.0 || ^14.17.0 || >=16.0.0" - }, - "funding": { - "url": "https://opencollective.com/eslint" - } - }, - "node_modules/@stylistic/eslint-plugin/node_modules/minimatch": { - "version": "9.0.5", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-9.0.5.tgz", - "integrity": "sha512-G6T0ZX48xgozx7587koeX9Ys2NYy6Gmv//P89sEte9V9whIapMNF4idKxnW2QtCcLiTWlb/wfCabAtAFWhhBow==", - "dev": true, - "license": "ISC", - "dependencies": { - "brace-expansion": "^2.0.1" - }, - "engines": { - "node": ">=16 || 14 >=14.17" - }, - "funding": { - "url": "https://github.com/sponsors/isaacs" - } - }, - "node_modules/@stylistic/eslint-plugin/node_modules/ts-api-utils": { - "version": "1.4.3", - "resolved": "https://registry.npmjs.org/ts-api-utils/-/ts-api-utils-1.4.3.tgz", - "integrity": "sha512-i3eMG77UTMD0hZhgRS562pv83RC6ukSAC2GMNWc+9dieh/+jDM5u5YG+NHX6VNDRHQcHwmsTHctP9LhbC3WxVw==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=16" - }, - "peerDependencies": { - "typescript": ">=4.2.0" - } - }, "node_modules/@tufjs/canonical-json": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/@tufjs/canonical-json/-/canonical-json-2.0.0.tgz", diff --git a/package.json b/package.json index 0d9d7c4eb..17c6c340f 100644 --- a/package.json +++ b/package.json @@ -88,7 +88,7 @@ "@eslint/js": "^9.15.0", "@istanbuljs/esm-loader-hook": "^0.3.0", "@istanbuljs/nyc-config-typescript": "^1.0.2", - "@stylistic/eslint-plugin": "^3.0.0", + "@stylistic/eslint-plugin": "^3.0.1", "@types/he": "^1.2.3", "@types/node": "20.11.0", "@types/sinon": "^17.0.3", diff --git a/src/linter/messages.ts b/src/linter/messages.ts index 1d2da9dfb..d4a232a3e 100644 --- a/src/linter/messages.ts +++ b/src/linter/messages.ts @@ -411,7 +411,7 @@ export const MESSAGE_INFO = { fatal: true, message: ({message}: {message: string}) => message, - details: () => `Check the source file for syntax errors`, + details: ({details}: {details?: string}) => details ? details : `Check the source file for syntax errors`, }, [MESSAGE.PARTIALLY_DEPRECATED_CORE_ROUTER]: { diff --git a/src/linter/ui5Types/SourceFileLinter.ts b/src/linter/ui5Types/SourceFileLinter.ts index 0640c45a0..4f35048e8 100644 --- a/src/linter/ui5Types/SourceFileLinter.ts +++ b/src/linter/ui5Types/SourceFileLinter.ts @@ -21,6 +21,8 @@ import type {ApiExtract} from "../../utils/ApiExtract.js"; import {findDirectives} from "./directives.js"; import BindingLinter from "../binding/BindingLinter.js"; import {RequireDeclaration} from "../xmlTemplate/Parser.js"; +import BindingParser from "../binding/lib/BindingParser.js"; +import {PropertyBindingInfo} from "sap/ui/base/ManagedObject"; const log = getLogger("linter:ui5Types:SourceFileLinter"); @@ -846,7 +848,7 @@ export default class SourceFileLinter { } else if (["bindProperty", "bindAggregation"].includes(symbolName) && moduleName === "sap/ui/base/ManagedObject" && node.arguments[1] && ts.isObjectLiteralExpression(node.arguments[1])) { - this.#analyzePropertyBindings(node.arguments[1], ["type"]); + this.#analyzePropertyBindings(node.arguments[1], ["type", "formatter"]); } else if (symbolName.startsWith("bind") && nodeType.symbol?.declarations?.some((declaration) => this.isUi5ClassDeclaration(declaration, "sap/ui/core/Control")) && @@ -857,7 +859,7 @@ export default class SourceFileLinter { const alternativePropName = propName.charAt(0).toLowerCase() + propName.slice(1); if (this.#isPropertyBinding(node, [propName, alternativePropName])) { - this.#analyzePropertyBindings(node.arguments[0], ["type"]); + this.#analyzePropertyBindings(node.arguments[0], ["type", "formatter"]); } } } @@ -1182,7 +1184,7 @@ export default class SourceFileLinter { this.#isPropertyBinding(node, [getPropertyNameText(prop.name) ?? ""])) ) { if (ts.isObjectLiteralExpression(prop.initializer)) { - this.#analyzePropertyBindings(prop.initializer, ["type"]); + this.#analyzePropertyBindings(prop.initializer, ["type", "formatter"]); } else { this.#analyzePropertyStringBindings(prop); } @@ -1196,7 +1198,7 @@ export default class SourceFileLinter { return; } - // Get the type property + // Get the property inside the binding let propertyField; if (ts.isObjectLiteralExpression(prop.initializer)) { propertyField = getPropertyAssignmentsInObjectLiteralExpression( @@ -1206,7 +1208,19 @@ export default class SourceFileLinter { // Whether it's a direct property of the Control // or name collision in property binding !ts.isNewExpression(prop.parent.parent)) { - propertyField = prop; + /* Special Case (JS/TS): If the value of the property 'formatter' is a string, + it should be detected since the runtime cannot resolve it + even if a 'formatter' variable is imported: */ + if (prop.name.getText() === "formatter") { + this.#reporter.addMessage(MESSAGE.PARSING_ERROR, { + message: `Value of property 'formatter' is of type 'string'.`, + details: + `Do not use strings for 'formatter' values in JavaScript and TypeScript. ` + + `{@link topic:28fcd55b04654977b63dacbee0552712 See Best Practices for Developers}`, + }, prop.initializer); + } else { + propertyField = prop; + } } if (propertyField) { @@ -1218,6 +1232,26 @@ export default class SourceFileLinter { #analyzePropertyStringBindings(node: ts.PropertyAssignment) { if (ts.isStringLiteralLike(node.initializer) && node.initializer.text.startsWith("{") && node.initializer.text.endsWith("}")) { + /* Special Case (JS/TS): If the value of the property 'formatter' is a string, + it should be detected since the runtime cannot resolve it + even if a 'formatter' variable is imported: */ + try { + const bindingInfo = BindingParser.complexParser(node.initializer.text, null, true, true, true, true); + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const {formatter} = bindingInfo as PropertyBindingInfo; + // Check if formatter is of type string and report a message if so: + if (formatter && typeof formatter === "string") { + this.#reporter.addMessage(MESSAGE.PARSING_ERROR, { + message: `Value of property 'formatter' is of type 'string'.`, + details: + `Do not use strings for 'formatter' values in JavaScript and TypeScript. ` + + `{@link topic:28fcd55b04654977b63dacbee0552712 See Best Practices for Developers}`, + }, node.initializer); + } + } catch { + // Ignore errors since they are already reported by the BindingParser + } + const imports = this.sourceFile.statements .filter((stmnt): stmnt is ts.ImportDeclaration => stmnt.kind === ts.SyntaxKind.ImportDeclaration) diff --git a/test/fixtures/linter/projects/com.ui5.troublesome.app/ui5lint-custom-broken.config.cjs b/test/fixtures/linter/projects/com.ui5.troublesome.app/ui5lint-custom-broken.config.cjs index fac283f5c..3ef957104 100644 --- a/test/fixtures/linter/projects/com.ui5.troublesome.app/ui5lint-custom-broken.config.cjs +++ b/test/fixtures/linter/projects/com.ui5.troublesome.app/ui5lint-custom-broken.config.cjs @@ -1,4 +1,7 @@ -module.exports = { - ignores: [ +// CodeQL code scan complains about this file being broken. +// This is a case we test for the config manager. So, wrapping it in a JSON.parse to avoid the error there, +// but keep the test case valid. +module.exports = JSON.parse(`{ + "ignores": [ "test/**/*", - "!test/sap/m/visual/Wizard.spe + "!test/sap/m/visual/Wizard.spe`); diff --git a/test/fixtures/linter/rules/NoGlobals/FormatterGlobalBindingObject.js b/test/fixtures/linter/rules/NoGlobals/FormatterGlobalBindingObject.js new file mode 100644 index 000000000..f22653b71 --- /dev/null +++ b/test/fixtures/linter/rules/NoGlobals/FormatterGlobalBindingObject.js @@ -0,0 +1,8 @@ +sap.ui.define( + ["sap/m/Input"], + (Input) => { + const input = new Input({ + value: { path: 'invoice>Status', formatter: 'formatter.statusText' } + }); + } +); diff --git a/test/fixtures/linter/rules/NoGlobals/FormatterGlobalBindingString.js b/test/fixtures/linter/rules/NoGlobals/FormatterGlobalBindingString.js new file mode 100644 index 000000000..b90dbd68d --- /dev/null +++ b/test/fixtures/linter/rules/NoGlobals/FormatterGlobalBindingString.js @@ -0,0 +1,18 @@ +sap.ui.define( + ["sap/m/Input", "ui5/walkthrough/model/formatter"], + (Input, formatter) => { + // The following two cases using global notations should be detected: + const input = new Input({ + value: "{ path: 'invoice>Status', formatter: 'ui5.walkthrough.model.formatter.statusText' }" + }); + input.applySettings({ + value: "{ path: 'invoice>Status', formatter: 'ui5.walkthrough.model.formatter.statusText' }", + }); + + // Although the formatter property does not look like a global notation, + // it should still be detected if it is a string: + const input2 = new Input({ + value: "{ path: 'invoice>Status', formatter: 'formatter.statusText' }" + }); + } +); diff --git a/test/lib/linter/rules/snapshots/NoGlobals.ts.md b/test/lib/linter/rules/snapshots/NoGlobals.ts.md index 9c2f6a393..a002680ad 100644 --- a/test/lib/linter/rules/snapshots/NoGlobals.ts.md +++ b/test/lib/linter/rules/snapshots/NoGlobals.ts.md @@ -4,6 +4,90 @@ The actual snapshot is saved in `NoGlobals.ts.snap`. Generated by [AVA](https://avajs.dev). +## General: FormatterGlobalBindingObject.js + +> Snapshot 1 + + [ + { + coverageInfo: [], + errorCount: 1, + fatalErrorCount: 1, + filePath: 'FormatterGlobalBindingObject.js', + messages: [ + { + column: 48, + fatal: true, + line: 5, + message: 'Value of property \'formatter\' is of type \'string\'.', + messageDetails: 'Do not use strings for \'formatter\' values in JavaScript and TypeScript. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)', + ruleId: 'parsing-error', + severity: 2, + }, + ], + warningCount: 0, + }, + ] + +## General: FormatterGlobalBindingString.js + +> Snapshot 1 + + [ + { + coverageInfo: [], + errorCount: 5, + fatalErrorCount: 3, + filePath: 'FormatterGlobalBindingString.js', + messages: [ + { + column: 4, + line: 6, + message: 'Access of global variable \'ui5\' (ui5.walkthrough.model.formatter.statusText)', + messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)', + ruleId: 'no-globals', + severity: 2, + }, + { + column: 4, + line: 9, + message: 'Access of global variable \'ui5\' (ui5.walkthrough.model.formatter.statusText)', + messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)', + ruleId: 'no-globals', + severity: 2, + }, + { + column: 11, + fatal: true, + line: 6, + message: 'Value of property \'formatter\' is of type \'string\'.', + messageDetails: 'Do not use strings for \'formatter\' values in JavaScript and TypeScript. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)', + ruleId: 'parsing-error', + severity: 2, + }, + { + column: 11, + fatal: true, + line: 9, + message: 'Value of property \'formatter\' is of type \'string\'.', + messageDetails: 'Do not use strings for \'formatter\' values in JavaScript and TypeScript. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)', + ruleId: 'parsing-error', + severity: 2, + }, + { + column: 11, + fatal: true, + line: 15, + message: 'Value of property \'formatter\' is of type \'string\'.', + messageDetails: 'Do not use strings for \'formatter\' values in JavaScript and TypeScript. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)', + ruleId: 'parsing-error', + severity: 2, + }, + ], + warningCount: 0, + }, + ] + ## General: ModelDataTypes.view.xml > Snapshot 1 diff --git a/test/lib/linter/rules/snapshots/NoGlobals.ts.snap b/test/lib/linter/rules/snapshots/NoGlobals.ts.snap index 2760498d4..020435ef3 100644 Binary files a/test/lib/linter/rules/snapshots/NoGlobals.ts.snap and b/test/lib/linter/rules/snapshots/NoGlobals.ts.snap differ