Fix vertical_parameter_alignment false positive with multi-byte function names#6747
Conversation
…e names `vertical_parameter_alignment` compared parameter alignment using UTF-8 byte columns. When characters before the parameters span more than one byte (such as a function name containing non-ASCII letters), the byte column of a visually aligned continuation parameter no longer matched the first parameter's byte column, producing a false violation. Convert each parameter column to a grapheme-cluster column before comparing, matching the approach already used by `collection_alignment`. Add a non-triggering regression example covering the reported signature. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Generated by 🚫 Danger |
|
Aren't multi-byte names evil? Wouldn't it be better to avoid such names, either by the current rule or some other rule, rather than hiding the problem? Is there a more useful example of a name, rather than one that only causes confusion between variations of the "c" character? |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Fair point on the example, so I swapped the homoglyph name for a realistic accented one (résuméBuilder); on the wider question, non-ASCII identifiers are legal Swift and common in non-English codebases, so this rule should measure alignment correctly regardless, and discouraging such names feels like |
SimplyDanny
left a comment
SimplyDanny
left a comment
There was a problem hiding this comment.
Thanks! Looks good to me.
I think that all rules should generally be able to deal with multi-byte characters in code, especially in Swift code where this totally allowed and sometimes common. They should be able to handle the difference between byte-count and visible character length. Unintentionally confusing characters is a different topic and could be addressed by a dedicated rule. Sounds reasonable to me to lint on non-ASCII characters that look like ones (for example). Feel free to open a "Rule Request" in a separate issue, @kapitoshka438. |
|
Thanks for the quick review and merge, glad it landed, and we'll be around to help with more fixes whenever they come up. |
4 participants
Fixes #5037.
vertical_parameter_alignmentcompared parameter columns by UTF-8 byte offset, so a function name with multi-byte characters (e.g. a non-ASCII letter) shifted the byte column of visually aligned continuation parameters and produced a false violation.This converts each parameter column to a grapheme-cluster column before comparing, the same approach
collection_alignmentalready uses. A non-triggering regression example covering the reported signature is added.Verified with
swift test --filter VerticalParameterAlignmentand the broader rule suite (--filter RulesTests); linting the issue's snippet now reports no violation while genuine misalignment is still flagged.🤖 Generated with Claude Code