Skip to content

Commit 3c6cfc4

Browse files
matz3matz3
committed
feat: Check all attributes for bindings in XML
XML templating processes all attributes and treats them as property bindings. In addition, we do not have information about custom controls and their metadata, so with this change, also attributes of custom controls are linted.
1 parent dd2d09f commit 3c6cfc4

File tree

10 files changed

+114
-29
lines changed

10 files changed

+114
-29
lines changed

‎src/linter/xmlTemplate/Parser.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -564,9 +564,7 @@ export default class Parser {
564564
line: prop.start.line + 1, // Add one to align with IDEs
565565
column: prop.start.column + 1,
566566
};
567-
if (this.#apiExtract.isProperty(symbolName, prop.name)) {
568-
this.#bindingLinter.lintPropertyBinding(prop.value, this.#requireDeclarations, position);
569-
} else if (this.#apiExtract.isAggregation(symbolName, prop.name)) {
567+
if (this.#apiExtract.isAggregation(symbolName, prop.name)) {
570568
this.#bindingLinter.lintAggregationBinding(prop.value, this.#requireDeclarations, position);
571569
} else if (this.#apiExtract.isEvent(symbolName, prop.name)) {
572570
// In XML templating, it's possible to have bindings in event handlers
@@ -660,6 +658,10 @@ export default class Parser {
660658
if (!isValidEventHandler && errorMessage) {
661659
this.#bindingLinter.reportParsingError(errorMessage, position);
662660
}
661+
} else {
662+
// XML templating processes all attributes as property bindings, so we don't need to check
663+
// whether the attribute is a property or not
664+
this.#bindingLinter.lintPropertyBinding(prop.value, this.#requireDeclarations, position);
663665
}
664666
}
665667
// This node declares a control

‎test/e2e/snapshots/compare-snapshots.ts.md‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ Generated by [AVA](https://avajs.dev).
514514
warningCount: 1,
515515
},
516516
{
517-
errorCount: 9,
517+
errorCount: 10,
518518
fatalErrorCount: 0,
519519
filePath: 'webapp/view/TemplatingTable.fragment.xml',
520520
messages: [
@@ -553,6 +553,13 @@ Generated by [AVA](https://avajs.dev).
553553
ruleId: 'no-ambiguous-event-handler',
554554
severity: 1,
555555
},
556+
{
557+
column: 20,
558+
line: 28,
559+
message: 'Access of global variable \'sap\' (sap.ui.model.odata.AnnotationHelper.getNavigationPath)',
560+
ruleId: 'no-globals',
561+
severity: 2,
562+
},
556563
{
557564
column: 13,
558565
line: 29,

‎test/e2e/snapshots/compare-snapshots.ts.snap‎

54 Bytes
Binary file not shown.

‎test/fixtures/linter/rules/NoGlobals/XMLTemplatingV2.view.xml‎

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -33,24 +33,30 @@
3333
</template:if>
3434
<template:else>
3535
<!--
36-
In XML templating the bindings are interpreted as property bindings and therefore can
37-
have multiple parts with formatters that provide the actual binding path for the aggregation binding.
38-
But as we cannot know whether it will be processed during templating or via normal XML view processing,
39-
we need to treat aggregation binding also as potential property bindings and run those checks as well.
36+
XML templating processes ALL attributes and treats them as property bindings,
37+
even for aggregations and events.
4038
-->
41-
<Table items="{
42-
parts: [
43-
{
44-
path: 'facet>Target',
45-
formatter: 'sap.ui.model.odata.AnnotationHelper.getNavigationPath'
46-
},
47-
{
48-
path: 'facet>Label',
49-
formatter: 'sap.ui.core.sample.ViewTemplate.scenario.Helper.doSomething'
50-
}
51-
],
52-
formatter: 'sap.ui.core.sample.ViewTemplate.scenario.Helper.combineParts'
53-
}" />
39+
<Table
40+
id="{ path: 'facet>Target', formatter: 'sap.ui.core.sample.ViewTemplate.scenario.Helper.getId' }"
41+
items="{
42+
parts: [
43+
{
44+
path: 'facet>Target',
45+
formatter: 'sap.ui.model.odata.AnnotationHelper.getNavigationPath'
46+
},
47+
{
48+
path: 'facet>Label',
49+
formatter: 'sap.ui.core.sample.ViewTemplate.scenario.Helper.doSomething'
50+
}
51+
],
52+
formatter: 'sap.ui.core.sample.ViewTemplate.scenario.Helper.combineParts'
53+
}"
54+
selectionChange="{ path: 'facet>Target', formatter: 'sap.ui.core.sample.ViewTemplate.scenario.Helper.getEventHandler' }"
55+
ariaLabelledBy="{ path: 'facet>Target', formatter: 'sap.ui.core.sample.ViewTemplate.scenario.Helper.getAriaLabelledBy' }"
56+
class="{ path: 'facet>Target', formatter: 'sap.ui.core.sample.ViewTemplate.scenario.Helper.getCssClasses' }"
57+
binding="{path: 'facet>Target', formatter: 'sap.ui.model.odata.AnnotationHelper.getNavigationPath'}"
58+
/>
59+
5460
</template:else>
5561

5662
</template:with>

‎test/lib/linter/rules/snapshots/NoGlobals.ts.md‎

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -837,7 +837,7 @@ Generated by [AVA](https://avajs.dev).
837837
[
838838
{
839839
coverageInfo: [],
840-
errorCount: 5,
840+
errorCount: 10,
841841
fatalErrorCount: 0,
842842
filePath: 'XMLTemplatingV2.view.xml',
843843
messages: [
@@ -858,29 +858,69 @@ Generated by [AVA](https://avajs.dev).
858858
severity: 2,
859859
},
860860
{
861-
column: 12,
861+
column: 6,
862+
line: 40,
863+
message: 'Access of global variable \'sap\' (sap.ui.core.sample.ViewTemplate.scenario.Helper.getId)',
864+
messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
865+
ruleId: 'no-globals',
866+
severity: 2,
867+
},
868+
{
869+
column: 6,
862870
line: 41,
863871
message: 'Access of global variable \'sap\' (sap.ui.core.sample.ViewTemplate.scenario.Helper.combineParts)',
864872
messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
865873
ruleId: 'no-globals',
866874
severity: 2,
867875
},
868876
{
869-
column: 12,
877+
column: 6,
870878
line: 41,
871879
message: 'Access of global variable \'sap\' (sap.ui.model.odata.AnnotationHelper.getNavigationPath)',
872880
messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
873881
ruleId: 'no-globals',
874882
severity: 2,
875883
},
876884
{
877-
column: 12,
885+
column: 6,
878886
line: 41,
879887
message: 'Access of global variable \'sap\' (sap.ui.core.sample.ViewTemplate.scenario.Helper.doSomething)',
880888
messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
881889
ruleId: 'no-globals',
882890
severity: 2,
883891
},
892+
{
893+
column: 6,
894+
line: 54,
895+
message: 'Access of global variable \'sap\' (sap.ui.core.sample.ViewTemplate.scenario.Helper.getEventHandler)',
896+
messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
897+
ruleId: 'no-globals',
898+
severity: 2,
899+
},
900+
{
901+
column: 6,
902+
line: 55,
903+
message: 'Access of global variable \'sap\' (sap.ui.core.sample.ViewTemplate.scenario.Helper.getAriaLabelledBy)',
904+
messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
905+
ruleId: 'no-globals',
906+
severity: 2,
907+
},
908+
{
909+
column: 6,
910+
line: 56,
911+
message: 'Access of global variable \'sap\' (sap.ui.core.sample.ViewTemplate.scenario.Helper.getCssClasses)',
912+
messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
913+
ruleId: 'no-globals',
914+
severity: 2,
915+
},
916+
{
917+
column: 6,
918+
line: 57,
919+
message: 'Access of global variable \'sap\' (sap.ui.model.odata.AnnotationHelper.getNavigationPath)',
920+
messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
921+
ruleId: 'no-globals',
922+
severity: 2,
923+
},
884924
],
885925
warningCount: 0,
886926
},

‎test/lib/linter/rules/snapshots/NoGlobals.ts.snap‎

255 Bytes
Binary file not shown.

‎test/lib/linter/snapshots/linter.ts.md‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1204,7 +1204,7 @@ Generated by [AVA](https://avajs.dev).
12041204
},
12051205
{
12061206
coverageInfo: [],
1207-
errorCount: 9,
1207+
errorCount: 10,
12081208
fatalErrorCount: 0,
12091209
filePath: 'webapp/view/TemplatingTable.fragment.xml',
12101210
messages: [
@@ -1248,6 +1248,14 @@ Generated by [AVA](https://avajs.dev).
12481248
ruleId: 'no-ambiguous-event-handler',
12491249
severity: 1,
12501250
},
1251+
{
1252+
column: 20,
1253+
line: 28,
1254+
message: 'Access of global variable \'sap\' (sap.ui.model.odata.AnnotationHelper.getNavigationPath)',
1255+
messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
1256+
ruleId: 'no-globals',
1257+
severity: 2,
1258+
},
12511259
{
12521260
column: 13,
12531261
line: 29,
@@ -1882,7 +1890,7 @@ Generated by [AVA](https://avajs.dev).
18821890
},
18831891
{
18841892
coverageInfo: [],
1885-
errorCount: 9,
1893+
errorCount: 10,
18861894
fatalErrorCount: 0,
18871895
filePath: 'webapp/view/TemplatingTable.fragment.xml',
18881896
messages: [
@@ -1921,6 +1929,13 @@ Generated by [AVA](https://avajs.dev).
19211929
ruleId: 'no-ambiguous-event-handler',
19221930
severity: 1,
19231931
},
1932+
{
1933+
column: 20,
1934+
line: 28,
1935+
message: 'Access of global variable \'sap\' (sap.ui.model.odata.AnnotationHelper.getNavigationPath)',
1936+
ruleId: 'no-globals',
1937+
severity: 2,
1938+
},
19241939
{
19251940
column: 13,
19261941
line: 29,
@@ -3595,7 +3610,7 @@ Generated by [AVA](https://avajs.dev).
35953610
},
35963611
{
35973612
coverageInfo: [],
3598-
errorCount: 9,
3613+
errorCount: 10,
35993614
fatalErrorCount: 0,
36003615
filePath: 'webapp/view/TemplatingTable.fragment.xml',
36013616
messages: [
@@ -3639,6 +3654,14 @@ Generated by [AVA](https://avajs.dev).
36393654
ruleId: 'no-ambiguous-event-handler',
36403655
severity: 1,
36413656
},
3657+
{
3658+
column: 20,
3659+
line: 28,
3660+
message: 'Access of global variable \'sap\' (sap.ui.model.odata.AnnotationHelper.getNavigationPath)',
3661+
messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
3662+
ruleId: 'no-globals',
3663+
severity: 2,
3664+
},
36423665
{
36433666
column: 13,
36443667
line: 29,

‎test/lib/linter/snapshots/linter.ts.snap‎

206 Bytes
Binary file not shown.

‎test/lib/snapshots/index.integration.ts.md‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,7 @@ Generated by [AVA](https://avajs.dev).
533533
},
534534
{
535535
coverageInfo: [],
536-
errorCount: 9,
536+
errorCount: 10,
537537
fatalErrorCount: 0,
538538
filePath: 'webapp/view/TemplatingTable.fragment.xml',
539539
messages: [
@@ -572,6 +572,13 @@ Generated by [AVA](https://avajs.dev).
572572
ruleId: 'no-ambiguous-event-handler',
573573
severity: 1,
574574
},
575+
{
576+
column: 20,
577+
line: 28,
578+
message: 'Access of global variable \'sap\' (sap.ui.model.odata.AnnotationHelper.getNavigationPath)',
579+
ruleId: 'no-globals',
580+
severity: 2,
581+
},
575582
{
576583
column: 13,
577584
line: 29,

‎test/lib/snapshots/index.integration.ts.snap‎

59 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)