Skip to content

Commit

Permalink
fix(compiler-core): improve member expression check
Browse files Browse the repository at this point in the history
fix #3910
  • Loading branch information
yyx990803 committed Jun 9, 2021
1 parent 9a5bdb1 commit bc100c5
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 9 deletions.
18 changes: 17 additions & 1 deletion packages/compiler-core/__tests__/utils.spec.ts
Expand Up @@ -81,9 +81,25 @@ test('isMemberExpression', () => {
expect(isMemberExpression('obj[arr[ret[bar]]]')).toBe(true)
expect(isMemberExpression('obj[arr[ret[bar]]].baz')).toBe(true)
expect(isMemberExpression('obj[1 + 1]')).toBe(true)
// should warning
expect(isMemberExpression(`obj[x[0]]`)).toBe(true)
expect(isMemberExpression('obj[1][2]')).toBe(true)
expect(isMemberExpression('obj[1][2].foo[3].bar.baz')).toBe(true)
expect(isMemberExpression(`a[b[c.d]][0]`)).toBe(true)

// strings
expect(isMemberExpression(`a['foo' + bar[baz]["qux"]]`)).toBe(true)

// multiline whitespaces
expect(isMemberExpression('obj \n .foo \n [bar \n + baz]')).toBe(true)
expect(isMemberExpression(`\n model\n.\nfoo \n`)).toBe(true)

// should fail
expect(isMemberExpression('a \n b')).toBe(false)
expect(isMemberExpression('obj[foo')).toBe(false)
expect(isMemberExpression('objfoo]')).toBe(false)
expect(isMemberExpression('obj[arr[0]')).toBe(false)
expect(isMemberExpression('obj[arr0]]')).toBe(false)
expect(isMemberExpression('123[a]')).toBe(false)
expect(isMemberExpression('a + b')).toBe(false)
expect(isMemberExpression('foo()')).toBe(false)

This comment has been minimized.

Copy link
@jacekkarczmarczyk

jacekkarczmarczyk Jun 9, 2021

Contributor

while this is ok to fail i don't think it's ok that foo().bar fails (actually I didn't check if it really fails but I suppose so, if somehow it doesn't mabye it deserves a test case), it's prefectly valid and works fine in Vue 2:

https://codepen.io/jkarczm/pen/XWMBvMY?editors=1010

})
2 changes: 1 addition & 1 deletion packages/compiler-core/src/transforms/vModel.ts
Expand Up @@ -41,7 +41,7 @@ export const transformModel: DirectiveTransform = (dir, node, context) => {
bindingType &&
bindingType !== BindingTypes.SETUP_CONST

if (!isMemberExpression(expString) && !maybeRef) {
if (!expString.trim() || (!isMemberExpression(expString) && !maybeRef)) {
context.onError(
createCompilerError(ErrorCodes.X_V_MODEL_MALFORMED_EXPRESSION, exp.loc)
)
Expand Down
67 changes: 60 additions & 7 deletions packages/compiler-core/src/utils.ts
Expand Up @@ -56,14 +56,67 @@ const nonIdentifierRE = /^\d|[^\$\w]/
export const isSimpleIdentifier = (name: string): boolean =>
!nonIdentifierRE.test(name)

const memberExpRE = /^[A-Za-z_$\xA0-\uFFFF][\w$\xA0-\uFFFF]*(?:\s*\.\s*[A-Za-z_$\xA0-\uFFFF][\w$\xA0-\uFFFF]*|\[(.+)\])*$/
const enum MemberExpLexState {
inMemberExp,
inBrackets,
inString
}

const validFirstIdentCharRE = /[A-Za-z_$\xA0-\uFFFF]/
const validIdentCharRE = /[\.\w$\xA0-\uFFFF]/
const whitespaceRE = /\s+[.[]\s*|\s*[.[]\s+/g

/**
* Simple lexer to check if an expression is a member expression. This is
* lax and only checks validity at the root level (i.e. does not validate exps
* inside square brackets), but it's ok since these are only used on template
* expressions and false positives are invalid expressions in the first place.
*/
export const isMemberExpression = (path: string): boolean => {
if (!path) return false
const matched = memberExpRE.exec(path.trim())
if (!matched) return false
if (!matched[1]) return true
if (!/[\[\]]/.test(matched[1])) return true
return isMemberExpression(matched[1].trim())
// remove whitespaces around . or [ first
path = path.trim().replace(whitespaceRE, s => s.trim())

let state = MemberExpLexState.inMemberExp
let prevState = MemberExpLexState.inMemberExp
let currentOpenBracketCount = 0
let currentStringType: "'" | '"' | '`' | null = null

for (let i = 0; i < path.length; i++) {
const char = path.charAt(i)
switch (state) {
case MemberExpLexState.inMemberExp:
if (char === '[') {
prevState = state
state = MemberExpLexState.inBrackets
currentOpenBracketCount++
} else if (
!(i === 0 ? validFirstIdentCharRE : validIdentCharRE).test(char)
) {
return false
}
break
case MemberExpLexState.inBrackets:
if (char === `'` || char === `"` || char === '`') {
prevState = state
state = MemberExpLexState.inString
currentStringType = char
} else if (char === `[`) {
currentOpenBracketCount++
} else if (char === `]`) {
if (!--currentOpenBracketCount) {
state = prevState
}
}
break
case MemberExpLexState.inString:
if (char === currentStringType) {
state = prevState
currentStringType = null
}
break
}
}
return !currentOpenBracketCount
}

export function getInnerRange(
Expand Down

0 comments on commit bc100c5

Please sign in to comment.