Skip to content

Commit

Permalink
feat: OData implicit globals detection (#510)
Browse files Browse the repository at this point in the history
JIRA: CPOUI5FOUNDATION-915
  • Loading branch information
d3xter666 authored Jan 31, 2025
1 parent 67e86d8 commit daff87b
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 9 deletions.
24 changes: 24 additions & 0 deletions src/linter/binding/BindingLinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,4 +152,28 @@ export default class BindingLinter {
this.#context.addLintingMessage(this.#resourcePath, MESSAGE.PARSING_ERROR, {message}, position);
}
}

#isExpressionBinding(bindingDefinition: string): boolean {
return /^\{:?=/.test(bindingDefinition) && bindingDefinition.endsWith("}");
}

lintPropertyExpression(
bindingDefinition: string, requireDeclarations: RequireDeclaration[], position: PositionInfo) {
if (!this.#isExpressionBinding(bindingDefinition)) {
return;
}
const allFunctionsModule = "sap/ui/model/odata/ODataExpressionAddons";
const varModuleMap = {
"odata.compare": "sap/ui/model/odata/v4/ODataUtils",
"odata.fillUriTemplate": "sap/ui/thirdparty/URITemplate",
"odata.uriEncode": "sap/ui/model/odata/ODataUtils",
};

for (const [key, value] of Object.entries(varModuleMap)) {
if (bindingDefinition.includes(`${key}(`) &&
!requireDeclarations.some((decl) => decl.moduleName === value || value === allFunctionsModule)) {
this.#context.addLintingMessage(this.#resourcePath, MESSAGE.NO_ODATA_GLOBALS, {} as never, position);
}
}
}
}
11 changes: 11 additions & 0 deletions src/linter/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export enum MESSAGE {
NO_EXPORTED_VALUES_BY_LIB,
NO_GLOBALS,
NO_ICON_POOL_RENDERER,
NO_ODATA_GLOBALS,
NOT_STATIC_CONTROL_RENDERER,
PARSING_ERROR,
PARTIALLY_DEPRECATED_CORE_ROUTER,
Expand Down Expand Up @@ -614,4 +615,14 @@ export const MESSAGE_INFO = {
`Please import the module itself directly instead of accessing it via the library module.`,
},

[MESSAGE.NO_ODATA_GLOBALS]: {
severity: LintMessageSeverity.Error,
ruleId: RULES["no-implicit-globals"],

message: () => "OData built-in global symbols must not be used implicitly",
details: () =>
"Import the 'sap/ui/model/odata/ODataExpressionAddons' module instead. " +
"See {@link topic:28fcd55b04654977b63dacbee0552712 Best Practices for Developers}",
},

} as const;
4 changes: 4 additions & 0 deletions src/linter/xmlTemplate/Parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,10 @@ export default class Parser {
line: prop.start.line + 1, // Add one to align with IDEs
column: prop.start.column + 1,
});
this.#bindingLinter.lintPropertyExpression(prop.value, this.#requireDeclarations, {
line: prop.start.line + 1, // Add one to align with IDEs
column: prop.start.column + 1,
});
} else if (this.#apiExtract.isAggregation(`${namespace}.${moduleName}`, prop.name)) {
this.#bindingLinter.lintAggregationBinding(prop.value, this.#requireDeclarations, {
line: prop.start.line + 1, // Add one to align with IDEs
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
<mvc:View xmlns="sap.m" xmlns:mvc="sap.ui.core.mvc" xmlns:core="sap.ui.core">
<mvc:View xmlns="sap.m" xmlns:mvc="sap.ui.core.mvc" xmlns:core="sap.ui.core" core:require="{
bar: 'sap/ui/model/odata/ODataUtils'
}">

<Input
value="{
path: 'amount',
Expand All @@ -12,4 +15,19 @@
core:require="{
Integer: 'sap/ui/model/type/Integer'
}"/>

<Text text="{= odata.compare(%{myvalue1},%{myvalue2})}" core:require="{
foo: 'sap/ui/model/odata/v4/ODataUtils'
}" />

<!-- "sap/ui/model/odata/ODataExpressionAddons" contains all of the odata globals -->
<Text text="{= odata.compare(${myvalue1},${path: 'myvalue1', model: 'myModel'})}" core:require="{
foo: 'sap/ui/model/odata/ODataExpressionAddons'
}" />

<!-- Should not be reported. Module already required with the previous binding -->
<Text text="{= odata.compare(%{myvalue1},%{myvalue2})}" />

<!-- Should not be reported. Module already required in the root -->
<Text text="{= odata.uriEncode(%{myvalue1})}" />
</mvc:View>
3 changes: 3 additions & 0 deletions test/fixtures/transpiler/xml/XMLViewBindings.view.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
xmlns:core="sap.ui.core"
controllerName="com.myapp.controller.Main"
>

<Text text="{= odata.compare(%{myvalue1},%{myvalue2})}" />

<!-- text: Deprecated reference of global formatter and binding event handler function -->
<!-- tooltip: Aggregation with a 0..1 aggregation and "string" alt type can be used like a property -->
<!-- customData: Aggregation with factory and nested filter -->
Expand Down
30 changes: 22 additions & 8 deletions test/lib/linter/xmlTemplate/snapshots/transpiler.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -1335,7 +1335,12 @@ Generated by [AVA](https://avajs.dev).
import ObjectStatus2 from "sap/m/ObjectStatus";␊
import ObjectStatus3 from "sap/m/ObjectStatus";␊
import ObjectStatus4 from "sap/m/ObjectStatus";␊
import Text from "sap/m/Text";␊
import View from "sap/ui/core/mvc/View";␊
const oText = new Text({␊
text: "{= odata.compare(%{myvalue1},%{myvalue2})}",␊
});␊
const oObjectStatus = new ObjectStatus({␊
text: "{\\n\\t\\t\\tpath: 'invoice>Status',\\n\\t\\t\\tformatter: 'ui5.walkthrough.model.formatter.statusText',\\n\\t\\t\\tevents: {\\n\\t\\t\\t\\tdataRequested: 'global.onMyDataRequested'\\n\\t\\t\\t}\\n\\t\\t}",␊
tooltip: "{\\n\\t\\t\\tpath: 'invoice>StatusDetails',\\n\\t\\t\\tformatter: 'globalTooltipFormatter'\\n\\t\\t}",␊
Expand All @@ -1359,6 +1364,7 @@ Generated by [AVA](https://avajs.dev).
export default const oView = new View({␊
controllerName: "com.myapp.controller.Main",␊
content: [␊
oText,␊
oObjectStatus,␊
oObjectStatus2,␊
oObjectStatus3,␊
Expand All @@ -1372,7 +1378,7 @@ Generated by [AVA](https://avajs.dev).
{
file: 'XMLViewBindings.view.js',
mappings: 'AAsDE,4CAKE;AALF,iCAKE;AALF,uCAKE;AALF,wDAKE;AAlDH,8CA6BE;AAGF,+CAKE;AAEF,+CAEE;AAGF,+CA6BE;AAlFH,wCAIC;sBAKA,kBA6BE;IA7BY,6LAMX;IAEF,qGAGE;IAEF,wVAcE;IACF,kCAAgC;;;uBAIjC,mBAKE;IAJD,6EAGE;;;uBAGH,mBAEE;IADD,kGAAwG;;;uBAIzG,mBA6BE;IAtBD,+LAME;IACF,iVAcE;;;6BAjFJ,UAIC;IADA,4CAA0C;IAM1C,OA6BE',
mappings: 'AAyDE,4CAKE;AALF,iCAKE;AALF,uCAKE;AALF,wDAKE;AAlDH,8CA6BE;AAGF,+CAKE;AAEF,+CAEE;AAGF,+CA6BE;AA/EF,8BAA0D;AAN3D,wCAIC;cAEA,UAA0D;IAApD,mDAAiD;;;sBAMvD,kBA6BE;IA7BY,6LAMX;IAEF,qGAGE;IAEF,wVAcE;IACF,kCAAgC;;;uBAIjC,mBAKE;IAJD,6EAGE;;;uBAGH,mBAEE;IADD,kGAAwG;;;uBAIzG,mBA6BE;IAtBD,+LAME;IACF,iVAcE;;;6BApFJ,UAIC;IADA,4CAA0C;IAG1C,OAA0D',
names: [],
sources: [
'XMLViewBindings.view.xml',
Expand All @@ -1383,57 +1389,65 @@ Generated by [AVA](https://avajs.dev).
> messages
[
{
column: 8,
line: 7,
message: 'OData built-in global symbols must not be used implicitly',
messageDetails: 'Import the \'sap/ui/model/odata/ODataExpressionAddons\' module instead. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
ruleId: 'no-implicit-globals',
severity: 2,
},
{
column: 16,
line: 10,
line: 13,
message: 'Access of global variable \'global\' (global.onMyDataRequested)',
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: 16,
line: 10,
line: 13,
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: 3,
line: 18,
line: 21,
message: 'Access of global variable \'globalTooltipFormatter\' (globalTooltipFormatter)',
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: 3,
line: 23,
line: 26,
message: 'Access of global variable \'global\' (global.factoryFunction)',
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: 3,
line: 23,
line: 26,
message: 'Access of global variable \'global\' (global.testMethod)',
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: 3,
line: 23,
line: 26,
message: 'Access of global variable \'global\' (global.testMethod)',
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: 3,
line: 43,
line: 46,
message: 'Access of global variable \'parseInt\' (parseInt)',
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',
Expand Down
Binary file modified test/lib/linter/xmlTemplate/snapshots/transpiler.ts.snap
Binary file not shown.

0 comments on commit daff87b

Please sign in to comment.