feat(lint): add noComposingEnterKey rule#10042
Conversation
🦋 Changeset detectedLatest commit: 998b8a2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
A-Project
Merging this PR will not alter performance
Comparing 
Footnotes
|
dyc3
left a comment
dyc3
left a comment
There was a problem hiding this comment.
I see a lot of .to_string() calls, those can likely be avoided with borrowed strings or TokenText.
FYI, the repo skills should have prevented a lot of issues, codex might not be picking up our skills because they are in .claude.
also, consider adding a custom migrator for the rule options. there's a skill for it to get the agent to do it.
| /// </form> | ||
| /// ``` | ||
| pub NoComposingEnterKey { | ||
| version: "2.4.12", |
There was a problem hiding this comment.
| version: "2.4.12", | |
| version: "next", |
| #[derive(Clone, Debug)] | ||
| enum DiagnosticKind { | ||
| MissingGuard(Box<str>), | ||
| DeprecatedKeypress(Box<str>), | ||
| MissingKeyCode229, | ||
| } | ||
|
|
||
| enum Handler { | ||
| Arrow(JsArrowFunctionExpression), | ||
| Function(JsFunctionExpression), | ||
| } | ||
|
|
||
| impl Handler { | ||
| fn body(&self) -> Option<JsSyntaxNode> { | ||
| match self { | ||
| Self::Arrow(handler) => Some(handler.body().ok()?.syntax().clone()), | ||
| Self::Function(handler) => Some(handler.body().ok()?.syntax().clone()), | ||
| } | ||
| } | ||
| } |
| /// ``` | ||
| pub NoComposingEnterKey { | ||
| version: "2.4.12", | ||
| name: "noComposingEnterKey", |
There was a problem hiding this comment.
I'm not totally sold on the name. no prefix means we are banning a concept, but actually we are enforcing that this guard is in place.
some ideas:
useComposingInputGuarduseComposingGuard
| JsArrowFunctionExpression::can_cast(node.kind()) | ||
| || JsFunctionExpression::can_cast(node.kind()) | ||
| || JsFunctionDeclaration::can_cast(node.kind()) | ||
| } |
There was a problem hiding this comment.
use this instead
biome/crates/biome_js_analyze/src/ast_utils.rs
Lines 372 to 375 in 85887c3
| #[serde(default = "default_check_key_code_for_safari")] | ||
| pub check_key_code_for_safari: bool, | ||
| #[serde(default)] | ||
| pub guard_functions: Vec<String>, |
There was a problem hiding this comment.
should be a boxed slice of str
| pub struct NoComposingEnterKeyOptions { | ||
| #[serde(default = "default_check_key_code_for_safari")] | ||
| pub check_key_code_for_safari: bool, | ||
| #[serde(default)] | ||
| pub guard_functions: Vec<String>, | ||
| } |
There was a problem hiding this comment.
These must all be Option for configuration merging to make sense
| impl Default for NoComposingEnterKeyOptions { | ||
| fn default() -> Self { | ||
| Self { | ||
| check_key_code_for_safari: default_check_key_code_for_safari(), | ||
| guard_functions: Vec::new(), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const fn default_check_key_code_for_safari() -> bool { | ||
| true | ||
| } |
There was a problem hiding this comment.
look at how other rules deal with default options. they don't access the options directly, they do it through a getter that calls unwrap_or to handle the Option
| } | ||
| } | ||
|
|
||
| fn is_key_code_229_binary_expression(binary: &JsBinaryExpression) -> bool { |
There was a problem hiding this comment.
consider returning Option<bool> to use the try operator
| fn is_enter_string_comparison(left: &AnyJsExpression, right: &AnyJsExpression) -> bool { | ||
| static_member_name(left) | ||
| .is_some_and(|member_name| matches!(member_name.as_ref(), "key" | "code")) | ||
| && string_literal_text(right).is_some_and(|text| text.as_ref() == "Enter") |
There was a problem hiding this comment.
more keys than just "Enter" matter right? like tab?
3 participants
Note
This PR was created with AI assistance (Codex).
Summary
Implements the new nursery rule
noComposingEnterKeydiscussed in #10041.The rule reports Enter-key submit handlers that do not guard against IME composition, including the Safari fallback with
keyCode === 229when enabled. The implementation was aligned witheslint-plugin-ime-safe-form, and its invalid/valid cases were migrated into Biome's spec fixtures.Test Plan
Added snapshot tests for DOM event listeners, JSX handlers,
onkeydown-style assignments,keypress,switch-based Enter checks, Safari fallback handling, andguardFunctionsoptions.Docs
This PR adds a new lint rule, so the documentation is included in the rule source and diagnostics. No separate website PR is needed.