Skip to content

Add new invisible_characters rule #6424

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.

Sign up for GitHub

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

Jump to bottom
Open
kapitoshka438 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add new invisible_characters rule #6424

kapitoshka438 wants to merge 3 commits into from

Conversation

@kapitoshka438
Copy link
Contributor

@kapitoshka438 kapitoshka438 commented Jan 8, 2026 •
edited
Loading

#6045

Summary

  • Adds new invisible_characters lint rule that detects invisible characters in string literals:
    • zero-width space U+200B
    • zero-width non-joiner U+200C
    • FEFF formatting character U+FEFF
  • Reports all occurrences with precise violation positions pointing to each invisible character
  • Includes early exit optimization to avoid unnecessary iteration when no invisible characters are present

@SwiftLintBot
Copy link

SwiftLintBot commented Jan 8, 2026 •
edited
Loading

19 Messages
📖 Building this branch resulted in a binary size of 27259.32 KiB vs 27232.8 KiB when built on main (0% larger).
📖 Linting Aerial with this PR took 0.82 s vs 0.8 s on main (2% slower).
📖 Linting Alamofire with this PR took 1.06 s vs 1.08 s on main (1% faster).
📖 Linting Brave with this PR took 7.3 s vs 7.28 s on main (0% slower).
📖 Linting DuckDuckGo with this PR took 24.68 s vs 24.68 s on main (0% slower).
📖 Linting Firefox with this PR took 11.79 s vs 11.77 s on main (0% slower).
📖 Linting Kickstarter with this PR took 8.07 s vs 8.06 s on main (0% slower).
📖 Linting Moya with this PR took 0.47 s vs 0.47 s on main (0% slower).
📖 Linting NetNewsWire with this PR took 2.51 s vs 2.47 s on main (1% slower).
📖 Linting Nimble with this PR took 0.72 s vs 0.7 s on main (2% slower).
📖 Linting PocketCasts with this PR took 7.43 s vs 7.49 s on main (0% faster).
📖 Linting Quick with this PR took 0.47 s vs 0.45 s on main (4% slower).
📖 Linting Realm with this PR took 3.38 s vs 3.34 s on main (1% slower).
📖 Linting Sourcery with this PR took 1.87 s vs 1.92 s on main (2% faster).
📖 Linting Swift with this PR took 4.52 s vs 4.47 s on main (1% slower).
📖 Linting SwiftLintPerformanceTests with this PR took 0.35 s vs 0.35 s on main (0% slower).
📖 Linting VLC with this PR took 1.18 s vs 1.14 s on main (3% slower).
📖 Linting Wire with this PR took 18.69 s vs 18.65 s on main (0% slower).
📖 Linting WordPress with this PR took 12.42 s vs 12.41 s on main (0% slower).

Generated by 🚫 Danger

@kapitoshka438 kapitoshka438 force-pushed the invisible_characters branch 2 times, most recently from 3561c44 to a89ae33 Compare January 8, 2026 21:52
@kapitoshka438
Copy link
Contributor Author

kapitoshka438 commented Jan 10, 2026

@SimplyDanny please review. I don't check NULL control character: \u0000 because it gets lost when copypasting so it's almost impossible to have it in string literals by accident.

@kapitoshka438 kapitoshka438 force-pushed the invisible_characters branch 2 times, most recently from 9382226 to ceccd1d Compare January 19, 2026 12:47
SimplyDanny
SimplyDanny requested changes Jan 23, 2026
View reviewed changes
Copy link
Collaborator

@SimplyDanny SimplyDanny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

I wonder if this rule should catch even more characters of this kind, some maybe optionally. We could check what other linters do.

@kapitoshka438 kapitoshka438 force-pushed the invisible_characters branch 3 times, most recently from 3a37574 to 708d32e Compare January 23, 2026 21:53
@kapitoshka438
Copy link
Contributor Author

kapitoshka438 commented Jan 23, 2026 •
edited
Loading

Thanks for the contribution!

I wonder if this rule should catch even more characters of this kind, some maybe optionally. We could check what other linters do.

  • This tool led me to another character (zero-width non-joiner U+200C). I've added processing it to this PR.
  • ESLint provides a rule that prevents using some control characters in regular expressions, but I think this is not our case because those characters are visible.
  • There is also zero-width joiner character U+200D, but it is used to construct complex symbols, such as some emoji example

SimplyDanny
SimplyDanny reviewed Jan 24, 2026
View reviewed changes
Copy link
Collaborator

@SimplyDanny SimplyDanny left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be wrong to have the rule auto-correct the findings by just removing the violating characters?

We could also think about making the rule configurable, so that people can add their own characters with descriptions. That's what the "tool" you cited allows as well.

@SimplyDanny SimplyDanny linked an issue Jan 25, 2026 that may be closed by this pull request
Rule Request: invisible_characters #6045
Open
2 tasks
@kapitoshka438
Copy link
Contributor Author

kapitoshka438 commented Jan 26, 2026

Would it be wrong to have the rule auto-correct the findings by just removing the violating characters?

We could also think about making the rule configurable, so that people can add their own characters with descriptions. That's what the "tool" you cited allows as well.

The second commit makes the rule correctable. I've also made some performance optimizations.
As for custom configurations, let me think about this. I'm sure there may be more symbols we haven't accounted for, but I'm not sure users should be forced to set descriptions for them. Each symbol has a name, and the purpose of this rule is to control the presence of specific special symbols, not just any symbols.

SimplyDanny
SimplyDanny reviewed Jan 27, 2026
View reviewed changes
Source/SwiftLintBuiltInRules/Rules/Lint/InvisibleCharacterRule.swift Outdated
ReasonedRuleViolation(
position: segment.content.positionAfterSkippingLeadingTrivia
.advanced(by: utf8Offset),
reason: "String literal should not contain invisible character \(characterName)"
Copy link
Collaborator

@SimplyDanny SimplyDanny Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrections could a bit easier to achieve. You can add a correction parameter here that replaces just ranges in the original code. It's simple but might be sufficient. Rewriters tend to turn quite complex very fast.

Tests/IntegrationTests/Resources/default_rule_configurations.yml Outdated
severity: error
meta:
opt-in: false
correctable: false
Copy link
Collaborator

@SimplyDanny SimplyDanny Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
correctable: false
correctable: true

Source/SwiftLintBuiltInRules/Rules/Lint/InvisibleCharacterRule.swift Outdated
"\u{FEFF}": "U+FEFF (zero-width no-break space)",
]

private static let invisibleCharacterSet: Set<Unicode.Scalar> = Set(invisibleCharacters.keys)
Copy link
Collaborator

@SimplyDanny SimplyDanny Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dictionaries are like sets internally. So I guess that checking whether the dictionary contains a key is as fast as using this separate set.

@SimplyDanny
Copy link
Collaborator

SimplyDanny commented Jan 27, 2026

I'm sure there may be more symbols we haven't accounted for, but I'm not sure users should be forced to set descriptions for them.

Fine for me to skip the description or have it optional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SimplyDanny SimplyDanny SimplyDanny requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Rule Request: invisible_characters

4 participants

@kapitoshka438 @SwiftLintBot @SimplyDanny @sm-eminiakhmetov