Skip to content

Commit

Permalink
refactor: Fix issue with escaped bindings
Browse files Browse the repository at this point in the history
  • Loading branch information
@matz3
matz3 committed Feb 12, 2025
1 parent 5f5cdc4 commit 062fb1d
Show file tree
Hide file tree
Showing 10 changed files with 119 additions and 41 deletions.
9 changes: 5 additions & 4 deletions src/linter/binding/BindingLinter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ export default class BindingLinter {
this.#context = context;
}

#parseBinding(binding: string): BindingInfo {
const bindingInfo = BindingParser.complexParser(binding, null, true, true, true, true);
#parseBinding(binding: string): BindingInfo | string | undefined {
// Do not unescape (3rd argument), as we're only interested in the binding
const bindingInfo = BindingParser.complexParser(binding, null, false, true, true, true);
return bindingInfo;
}

Expand Down Expand Up @@ -138,7 +139,7 @@ export default class BindingLinter {
lintPropertyBinding(bindingDefinition: string, requireDeclarations: RequireDeclaration[], position: PositionInfo) {
try {
const bindingInfo = this.#parseBinding(bindingDefinition);
if (bindingInfo) {
if (typeof bindingInfo === "object") {
this.#analyzePropertyBinding(bindingInfo as PropertyBindingInfo, requireDeclarations, position);
if ("tokens" in bindingInfo) {
this.#lintExpressionBindingTokens(bindingInfo, requireDeclarations, position);
Expand All @@ -154,7 +155,7 @@ export default class BindingLinter {
bindingDefinition: string, requireDeclarations: RequireDeclaration[], position: PositionInfo) {
try {
const bindingInfo = this.#parseBinding(bindingDefinition);
if (bindingInfo) {
if (typeof bindingInfo === "object") {
this.#analyzeAggregationBinding(bindingInfo as AggregationBindingInfo, requireDeclarations, position);
}
} catch (err) {
Expand Down
2 changes: 1 addition & 1 deletion src/linter/binding/lib/BindingParser.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ interface BindingParser {
bPreferContext?: boolean,
mLocals?: Record<string, string>,
bResolveTypesAsync?: boolean
) => BindingInfo;
) => BindingInfo | string | undefined; // Might return a string when bUnescape is true
}

declare const BindingParser: BindingParser;
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/linter/rules/NoGlobals/NoImplicitGlobalsOData.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ sap.ui.define(["sap/m/Text"], (Text) => {
const oText2 = new Text({
text: "{= odata.fillUriTemplate(${myvalue1},${myvalue2})}"
});
const oText3 = new Text({
oText2.applySettings({
text: "{= odata.uriEncode(%{myvalue1},'Edm.String') + ' - ' + odata.uriEncode(%{myvalue2},'Edm.String') }"
});
});
5 changes: 4 additions & 1 deletion test/fixtures/linter/rules/NoGlobals/NoImplicitGlobalsOData_Negative.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ sap.ui.define(["sap/m/Text", "sap/ui/model/odata/ODataExpressionAddons"], (Text)
const oText2 = new Text({
text: "{= odata.fillUriTemplate(${myvalue1},${myvalue2})}"
});
const oText3 = new Text({
oText2.applySettings({
text: "{= odata.uriEncode(%{myvalue1},'Edm.String') + ' - ' + odata.uriEncode(%{myvalue2},'Edm.String') }"
});
oText2.applySettings({
text: `\\{"foo": "bar"}` // Escaped JSON string should not cause issues when checking for odata globals within a binding
});
});
20 changes: 20 additions & 0 deletions test/lib/linter/binding/BindingLinter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -386,3 +386,23 @@ test("Error Testing: XML Property Binding missing closing bracket", (t) => {

t.snapshot(linterContext.generateLintResult("/test.js"));
});

test("Error Testing: Escaped JSON (escaped opening bracket)", (t) => {
const {bindingLinter, linterContext} = t.context;

bindingLinter.lintPropertyBinding(
`\\{"foo": "bar"}`,
[], {line: 1, column: 1});

t.snapshot(linterContext.generateLintResult("/test.js"));
});

test("Error Testing: Escaped JSON (escaped opening/closing brackets)", (t) => {
const {bindingLinter, linterContext} = t.context;

bindingLinter.lintPropertyBinding(
`\\{"foo": "bar"\\}`,
[], {line: 1, column: 1});

t.snapshot(linterContext.generateLintResult("/test.js"));
});
32 changes: 29 additions & 3 deletions test/lib/linter/binding/lib/BindingParser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import anyTest, {TestFn} from "ava";
import BindingParser, {BindingInfo} from "../../../../../src/linter/binding/lib/BindingParser.js";

const test = anyTest as TestFn<{
parse: (string: string) => BindingInfo;
parse: (string: string) => BindingInfo | string | undefined;
}>;

test.before((t) => {
t.context.parse = function (string: string) {
return BindingParser.complexParser(string, null, true, true, true, true);
return BindingParser.complexParser(string, null, false, true, true, true);
};
});

Expand Down Expand Up @@ -108,11 +108,37 @@ test("XML Binding: Expression Binding with an embedded composite binding", (t) =
const {parse} = t.context;

const res = parse(`{= %{/data/message}.length < 20
? %{i18n>errorMsg}
? %{i18n>errorMsg}
: %{parts: [
{path: 'i18n>successMsg'},
{path: '/data/today', type:'sap.ui.model.type.Date', constraints:{displayFormat:'Date'}},
{path: '/data/tomorrow', type:'sap.ui.model.type.Date', constraints:{displayFormat:'Date'}}
], formatter: 'my.globalFormatter'}}`);
t.snapshot(res);
});

test("No binding: Just text", (t) => {
const {parse} = t.context;

const res = parse(`foo bar`);
t.snapshot(res);
});

test("No binding: Escaped JSON (escaped opening bracket)", (t) => {
const {parse} = t.context;

const res = parse(`\\{"foo": "bar"}`);
t.snapshot(res);
});

test("No binding: Escaped JSON (escaped opening/closing brackets)", (t) => {
const {parse} = t.context;

const res = parse(`\\{"foo": "bar"\\}`);
t.snapshot(res);
});

test("No binding: Escaped JSON (escaped opening/closing brackets), bUnescape=true", (t) => {
const res = BindingParser.complexParser(`\\{"foo": "bar"\\}`, null, /* bUnescape */ true, true, true, true);
t.snapshot(res);
});
56 changes: 40 additions & 16 deletions test/lib/linter/binding/lib/snapshots/BindingParser.ts.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ Generated by [AVA](https://avajs.dev).
end: 19,
id: 'BINDING',
input: `{= %{/data/message}.length < 20␊
? %{i18n>errorMsg} ␊
? %{i18n>errorMsg}␊
: %{parts: [␊
{path: 'i18n>successMsg'},␊
{path: '/data/today', type:'sap.ui.model.type.Date', constraints:{displayFormat:'Date'}},␊
Expand All @@ -217,7 +217,7 @@ Generated by [AVA](https://avajs.dev).
end: 20,
id: '.',
input: `{= %{/data/message}.length < 20␊
? %{i18n>errorMsg} ␊
? %{i18n>errorMsg}␊
: %{parts: [␊
{path: 'i18n>successMsg'},␊
{path: '/data/today', type:'sap.ui.model.type.Date', constraints:{displayFormat:'Date'}},␊
Expand All @@ -229,7 +229,7 @@ Generated by [AVA](https://avajs.dev).
end: 26,
id: 'IDENTIFIER',
input: `{= %{/data/message}.length < 20␊
? %{i18n>errorMsg} ␊
? %{i18n>errorMsg}␊
: %{parts: [␊
{path: 'i18n>successMsg'},␊
{path: '/data/today', type:'sap.ui.model.type.Date', constraints:{displayFormat:'Date'}},␊
Expand All @@ -242,7 +242,7 @@ Generated by [AVA](https://avajs.dev).
end: 28,
id: '<',
input: `{= %{/data/message}.length < 20␊
? %{i18n>errorMsg} ␊
? %{i18n>errorMsg}␊
: %{parts: [␊
{path: 'i18n>successMsg'},␊
{path: '/data/today', type:'sap.ui.model.type.Date', constraints:{displayFormat:'Date'}},␊
Expand All @@ -254,7 +254,7 @@ Generated by [AVA](https://avajs.dev).
end: 31,
id: 'CONSTANT',
input: `{= %{/data/message}.length < 20␊
? %{i18n>errorMsg} ␊
? %{i18n>errorMsg}␊
: %{parts: [␊
{path: 'i18n>successMsg'},␊
{path: '/data/today', type:'sap.ui.model.type.Date', constraints:{displayFormat:'Date'}},␊
Expand All @@ -267,7 +267,7 @@ Generated by [AVA](https://avajs.dev).
end: 39,
id: '?',
input: `{= %{/data/message}.length < 20␊
? %{i18n>errorMsg} ␊
? %{i18n>errorMsg}␊
: %{parts: [␊
{path: 'i18n>successMsg'},␊
{path: '/data/today', type:'sap.ui.model.type.Date', constraints:{displayFormat:'Date'}},␊
Expand All @@ -279,7 +279,7 @@ Generated by [AVA](https://avajs.dev).
end: 56,
id: 'BINDING',
input: `{= %{/data/message}.length < 20␊
? %{i18n>errorMsg} ␊
? %{i18n>errorMsg}␊
: %{parts: [␊
{path: 'i18n>successMsg'},␊
{path: '/data/today', type:'sap.ui.model.type.Date', constraints:{displayFormat:'Date'}},␊
Expand All @@ -289,41 +289,65 @@ Generated by [AVA](https://avajs.dev).
value: 1,
},
{
end: 65,
end: 64,
id: ':',
input: `{= %{/data/message}.length < 20␊
? %{i18n>errorMsg} ␊
? %{i18n>errorMsg}␊
: %{parts: [␊
{path: 'i18n>successMsg'},␊
{path: '/data/today', type:'sap.ui.model.type.Date', constraints:{displayFormat:'Date'}},␊
{path: '/data/tomorrow', type:'sap.ui.model.type.Date', constraints:{displayFormat:'Date'}}␊
], formatter: 'my.globalFormatter'}}`,
start: 64,
start: 63,
},
{
end: 354,
end: 353,
id: 'BINDING',
input: `{= %{/data/message}.length < 20␊
? %{i18n>errorMsg} ␊
? %{i18n>errorMsg}␊
: %{parts: [␊
{path: 'i18n>successMsg'},␊
{path: '/data/today', type:'sap.ui.model.type.Date', constraints:{displayFormat:'Date'}},␊
{path: '/data/tomorrow', type:'sap.ui.model.type.Date', constraints:{displayFormat:'Date'}}␊
], formatter: 'my.globalFormatter'}}`,
start: 66,
start: 65,
value: 2,
},
{
end: 355,
end: 354,
id: '}',
input: `{= %{/data/message}.length < 20␊
? %{i18n>errorMsg} ␊
? %{i18n>errorMsg}␊
: %{parts: [␊
{path: 'i18n>successMsg'},␊
{path: '/data/today', type:'sap.ui.model.type.Date', constraints:{displayFormat:'Date'}},␊
{path: '/data/tomorrow', type:'sap.ui.model.type.Date', constraints:{displayFormat:'Date'}}␊
], formatter: 'my.globalFormatter'}}`,
start: 354,
start: 353,
},
],
}

## No binding: Just text

> Snapshot 1
undefined

## No binding: Escaped JSON (escaped opening bracket)

> Snapshot 1
undefined

## No binding: Escaped JSON (escaped opening/closing brackets)

> Snapshot 1
undefined

## No binding: Escaped JSON (escaped opening/closing brackets), bUnescape=true

> Snapshot 1
'{"foo": "bar"}'
Binary file modified test/lib/linter/binding/lib/snapshots/BindingParser.ts.snap
View file
Binary file not shown.
34 changes: 19 additions & 15 deletions test/lib/linter/binding/snapshots/BindingLinter.ts.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ Generated by [AVA](https://avajs.dev).
warningCount: 0,
}

## Error Testing: XML Expression Binding with syntax error
## Error Testing: XML Property Binding missing closing bracket

> Snapshot 1
Expand All @@ -477,32 +477,36 @@ Generated by [AVA](https://avajs.dev).
column: 1,
fatal: true,
line: 1,
message: 'Expected IDENTIFIER but instead saw (',
message: 'Expected \',\' instead of \'\'',
ruleId: 'parsing-error',
severity: 2,
},
],
warningCount: 0,
}

## Error Testing: XML Property Binding missing closing bracket
## Error Testing: Escaped JSON (escaped opening bracket)

> Snapshot 1
{
coverageInfo: [],
errorCount: 1,
fatalErrorCount: 1,
errorCount: 0,
fatalErrorCount: 0,
filePath: '/test.js',
messages: [
{
column: 1,
fatal: true,
line: 1,
message: 'Expected \',\' instead of \'\'',
ruleId: 'parsing-error',
severity: 2,
},
],
messages: [],
warningCount: 0,
}

## Error Testing: Escaped JSON (escaped opening/closing brackets)

> Snapshot 1
{
coverageInfo: [],
errorCount: 0,
fatalErrorCount: 0,
filePath: '/test.js',
messages: [],
warningCount: 0,
}
Binary file modified test/lib/linter/binding/snapshots/BindingLinter.ts.snap
View file
Binary file not shown.

0 comments on commit 062fb1d

Please sign in to comment.