-
-
Notifications
You must be signed in to change notification settings - Fork 8.9k
fix(compiler-core): handle unexpected characters in attribute names #13952
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdded a dev/non-browser pre-check in handleAttrStart to emit UNEXPECTED_CHARACTER_IN_ATTRIBUTE_NAME for stray Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant P as Parser
participant T as Tokenizer
participant E as ErrorHandler
rect rgba(245,250,255,0.9)
note over P,T: handleAttrStart invoked with currentChar
P->>T: handleAttrStart(currentChar)
end
alt currentChar is `"` / `'` / `<` (dev or non-browser)
T-->>E: emit UNEXPECTED_CHARACTER_IN_ATTRIBUTE_NAME
else currentChar is directive / dir-name / attr-name start
T->>T: continue existing branch (InDirName / directive / InAttrName)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
Comment |
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
@coderabbitai fix broken unit tests |
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed:
Length of output: 1656 🏁 Script executed:
Length of output: 220 🏁 Script executed:
Length of output: 269 🏁 Script executed:
Length of output: 28 🏁 Script executed:
Length of output: 505 🏁 Script executed:
Length of output: 62 🏁 Script executed:
Length of output: 64 🏁 Script executed:
Length of output: 0 🏁 Script executed:
Length of output: 21 🏁 Script executed:
Length of output: 34017 🏁 Script executed:
Length of output: 621 🏁 Script executed:
Length of output: 1103 🏁 Script executed:
Length of output: 20172 🏁 Script executed:
Length of output: 74 🏁 Script executed:
Length of output: 24905 🏁 Script executed:
Length of output: 48947 @linzhe141, I found the issue! Two unit tests are failing in The ProblemThe tests for
The FixYou need to update these two test cases to expect the additional errors. Here's what to do: Once I see the current test structure, I can provide you with the exact code changes needed to fix the tests. The solution will be to add the additional expected errors to these two test cases. 🏁 Script executed:
Length of output: 561 Perfect! Now I can provide you with the exact fix. Here's what you need to do: Fix the TestsUpdate the test cases in UNEXPECTED_EQUALS_SIGN_BEFORE_ATTRIBUTE_NAME: [
{
code: '<template><div =foo=bar></div></template>',
errors: [
{
type: ErrorCodes.UNEXPECTED_EQUALS_SIGN_BEFORE_ATTRIBUTE_NAME,
loc: { offset: 15, line: 1, column: 16 },
},
{
type: ErrorCodes.UNEXPECTED_CHARACTER_IN_ATTRIBUTE_NAME,
loc: { offset: 19, line: 1, column: 20 },
},
],
},
{
code: '<template><div =></div></template>',
errors: [
{
type: ErrorCodes.UNEXPECTED_EQUALS_SIGN_BEFORE_ATTRIBUTE_NAME,
loc: { offset: 15, line: 1, column: 16 },
},
{
type: ErrorCodes.MISSING_ATTRIBUTE_VALUE,
loc: { offset: 16, line: 1, column: 17 },
},
],
},
], Update SnapshotsAfter making the code changes above, run this command from the repository root to update the snapshots: pnpm test -- -u packages/compiler-core/__tests__/parse.spec.ts This will:
SummaryYour tokenizer fix is working correctly! It's now catching additional parsing errors that weren't being detected before. The test failures are simply because the test expectations need to be updated to reflect this improved error detection. After making these changes and updating the snapshots, all tests should pass. |
"offset": 23, | ||
}, | ||
"source": "=foo=bar", | ||
"source": "foo=bar", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure how these values in props
are used, but if the existing snapshots are going to be changed then I think the change needs to be explained. Was the previous snapshot value for source
incorrect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When attr compilation fails, the extracted props don’t really make sense, so I just updated the snapshot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be modified; the AST should preserve the original content.
close #13944
Based on the previous behavior of the vue compiler, this should be a bug
https://element-plus.run/#eyJBcHAudnVlIjoiPHRlbXBsYXRlPlxuICA8ZGl2IFwiPmhpPC9kaXY+XG48L3RlbXBsYXRlPlxuIiwiZWxlbWVudC1wbHVzLmpzIjoiaW1wb3J0IEVsZW1lbnRQbHVzIGZyb20gJ2VsZW1lbnQtcGx1cydcbmltcG9ydCB7IGdldEN1cnJlbnRJbnN0YW5jZSB9IGZyb20gJ3Z1ZSdcblxubGV0IGluc3RhbGxlZCA9IGZhbHNlXG5hd2FpdCBsb2FkU3R5bGUoKVxuXG5leHBvcnQgZnVuY3Rpb24gc2V0dXBFbGVtZW50UGx1cygpIHtcbiAgaWYgKGluc3RhbGxlZCkgcmV0dXJuXG4gIGNvbnN0IGluc3RhbmNlID0gZ2V0Q3VycmVudEluc3RhbmNlKClcbiAgaW5zdGFuY2UuYXBwQ29udGV4dC5hcHAudXNlKEVsZW1lbnRQbHVzKVxuICBpbnN0YWxsZWQgPSB0cnVlXG59XG5cbmV4cG9ydCBmdW5jdGlvbiBsb2FkU3R5bGUoKSB7XG4gIGNvbnN0IHN0eWxlcyA9IFsnaHR0cHM6Ly9jZG4uanNkZWxpdnIubmV0L25wbS9lbGVtZW50LXBsdXNAbGF0ZXN0L2Rpc3QvaW5kZXguY3NzJywgJ2h0dHBzOi8vY2RuLmpzZGVsaXZyLm5ldC9ucG0vZWxlbWVudC1wbHVzQGxhdGVzdC90aGVtZS1jaGFsay9kYXJrL2Nzcy12YXJzLmNzcyddLm1hcCgoc3R5bGUpID0+IHtcbiAgICByZXR1cm4gbmV3IFByb21pc2UoKHJlc29sdmUsIHJlamVjdCkgPT4ge1xuICAgICAgY29uc3QgbGluayA9IGRvY3VtZW50LmNyZWF0ZUVsZW1lbnQoJ2xpbmsnKVxuICAgICAgbGluay5yZWwgPSAnc3R5bGVzaGVldCdcbiAgICAgIGxpbmsuaHJlZiA9IHN0eWxlXG4gICAgICBsaW5rLmFkZEV2ZW50TGlzdGVuZXIoJ2xvYWQnLCByZXNvbHZlKVxuICAgICAgbGluay5hZGRFdmVudExpc3RlbmVyKCdlcnJvcicsIHJlamVjdClcbiAgICAgIGRvY3VtZW50LmJvZHkuYXBwZW5kKGxpbmspXG4gICAgfSlcbiAgfSlcbiAgcmV0dXJuIFByb21pc2UuYWxsU2V0dGxlZChzdHlsZXMpXG59XG4iLCJ0c2NvbmZpZy5qc29uIjoie1xuICBcImNvbXBpbGVyT3B0aW9uc1wiOiB7XG4gICAgXCJ0YXJnZXRcIjogXCJFU05leHRcIixcbiAgICBcImpzeFwiOiBcInByZXNlcnZlXCIsXG4gICAgXCJtb2R1bGVcIjogXCJFU05leHRcIixcbiAgICBcIm1vZHVsZVJlc29sdXRpb25cIjogXCJCdW5kbGVyXCIsXG4gICAgXCJ0eXBlc1wiOiBbXCJlbGVtZW50LXBsdXMvZ2xvYmFsLmQudHNcIl0sXG4gICAgXCJhbGxvd0ltcG9ydGluZ1RzRXh0ZW5zaW9uc1wiOiB0cnVlLFxuICAgIFwiYWxsb3dKc1wiOiB0cnVlLFxuICAgIFwiY2hlY2tKc1wiOiB0cnVlXG4gIH0sXG4gIFwidnVlQ29tcGlsZXJPcHRpb25zXCI6IHtcbiAgICBcInRhcmdldFwiOiAzLjNcbiAgfVxufVxuIiwiUGxheWdyb3VuZE1haW4udnVlIjoiPHNjcmlwdCBzZXR1cD5cbmltcG9ydCBBcHAgZnJvbSAnLi9BcHAudnVlJ1xuaW1wb3J0IHsgc2V0dXBFbGVtZW50UGx1cyB9IGZyb20gJy4vZWxlbWVudC1wbHVzLmpzJ1xuc2V0dXBFbGVtZW50UGx1cygpXG48L3NjcmlwdD5cblxuPHRlbXBsYXRlPlxuICA8QXBwIC8+XG48L3RlbXBsYXRlPlxuIiwiaW1wb3J0LW1hcC5qc29uIjoie1xuICBcImltcG9ydHNcIjoge1xuICAgIFwidnVlXCI6IFwiaHR0cHM6Ly9jZG4uanNkZWxpdnIubmV0L25wbS9AdnVlL3J1bnRpbWUtZG9tQDMuNC4wLWFscGhhLjEvZGlzdC9ydW50aW1lLWRvbS5lc20tYnJvd3Nlci5qc1wiLFxuICAgIFwiQHZ1ZS9zaGFyZWRcIjogXCJodHRwczovL2Nkbi5qc2RlbGl2ci5uZXQvbnBtL0B2dWUvc2hhcmVkQDMuNC4wLWFscGhhLjEvZGlzdC9zaGFyZWQuZXNtLWJ1bmRsZXIuanNcIixcbiAgICBcImVsZW1lbnQtcGx1c1wiOiBcImh0dHBzOi8vY2RuLmpzZGVsaXZyLm5ldC9ucG0vZWxlbWVudC1wbHVzQGxhdGVzdC9kaXN0L2luZGV4LmZ1bGwubWluLm1qc1wiLFxuICAgIFwiZWxlbWVudC1wbHVzL1wiOiBcImh0dHBzOi8vY2RuLmpzZGVsaXZyLm5ldC9ucG0vZWxlbWVudC1wbHVzQGxhdGVzdC9cIixcbiAgICBcIkBlbGVtZW50LXBsdXMvaWNvbnMtdnVlXCI6IFwiaHR0cHM6Ly9jZG4uanNkZWxpdnIubmV0L25wbS9AZWxlbWVudC1wbHVzL2ljb25zLXZ1ZUAyL2Rpc3QvaW5kZXgubWluLmpzXCJcbiAgfSxcbiAgXCJzY29wZXNcIjoge31cbn0iLCJDb21wLnZ1ZSI6IiIsIl9vIjp7InZ1ZVZlcnNpb24iOiIzLjQuMC1hbHBoYS4xIn19
Summary by CodeRabbit
Bug Fixes
Tests