Add new invisible_characters rule

[kapitoshka438]

#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]

	19 Messages
📖	Building this branch resulted in a binary size of 27476.09 KiB vs 27406.26 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.05 s vs 1.05 s on main (0% slower).
📖	Linting Brave with this PR took 7.27 s vs 7.24 s on main (0% slower).
📖	Linting DuckDuckGo with this PR took 27.67 s vs 27.73 s on main (0% faster).
📖	Linting Firefox with this PR took 12.16 s vs 12.18 s on main (0% faster).
📖	Linting Kickstarter with this PR took 8.45 s vs 8.42 s on main (0% slower).
📖	Linting Moya with this PR took 0.44 s vs 0.47 s on main (6% faster).
📖	Linting NetNewsWire with this PR took 2.64 s vs 2.61 s on main (1% slower).
📖	Linting Nimble with this PR took 0.72 s vs 0.69 s on main (4% slower).
📖	Linting PocketCasts with this PR took 7.76 s vs 7.99 s on main (2% faster).
📖	Linting Quick with this PR took 0.45 s vs 0.46 s on main (2% faster).
📖	Linting Realm with this PR took 3.0 s vs 2.98 s on main (0% slower).
📖	Linting Sourcery with this PR took 1.86 s vs 1.87 s on main (0% faster).
📖	Linting Swift with this PR took 4.72 s vs 4.71 s on main (0% slower).
📖	Linting SwiftLintPerformanceTests with this PR took 0.35 s vs 0.31 s on main (12% slower).
📖	Linting VLC with this PR took 1.24 s vs 1.26 s on main (1% faster).
📖	Linting Wire with this PR took 18.87 s vs 18.95 s on main (0% faster).
📖	Linting WordPress with this PR took 12.81 s vs 12.8 s on main (0% slower).

Generated by 🚫 Danger

[kapitoshka438]

@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.

[SimplyDanny]

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]

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]

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.

[kapitoshka438]

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]

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.

[kapitoshka438]

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.

I've added the ability to configure additional characters. I thought about it for a long time and initially decided not to allow disabling the validation of default characters. The hardest part was coming up with names for the parameter and variables 😅. But I'm still having doubts. Please review )

[SimplyDanny]

This looks very good now! Thank you for all the updates.

I posted a suggestion for the option's name. Let me know what you think about it.

[SimplyDanny]

There is something wrong with the tests on Linux and Windows. I can reproduce that on Linux. The test execution is just stuck.

[kapitoshka438]

There is something wrong with the tests on Linux and Windows. I can reproduce that on Linux. The test execution is just stuck.

I can reproduce that on Windows but I wasn't able to debug the issue. The problem is related only to the 0x200D character. For some reason, it just stucks on Windows/Linux if there is an Example with this character.
I tried turning off testMultiByteOffsets, testOnLinux, testOnWindows, setting excludeFromDocumentation to true. Nothing helps.

[SimplyDanny]

So that might even be an issue with the compiler. If we were able to create a small reproducer, we could use that to report a bug.

[kapitoshka438]

So that might even be an issue with the compiler. If we were able to create a small reproducer, we could use that to report a bug.

I was able to fix the behavior for Windows/Linux. On Windows, when ZWJ forms a grapheme cluster with the previous character (in this case "o" + ZWJ = one grapheme cluster), the
NSString.replacingCharacters(in:with:) operation deletes the entire cluster, not just the specified code unit.
Solution: Added a check in InvisibleCharacterRule.swift: if a violating character (e.g., ZWJ) forms a grapheme cluster with the previous character, then:

• The correction range is expanded to include the previous character
• The previous character is used as the replacement (instead of an empty string).

Here is a small piece of code where you can see the difference between Mac OS and Windows/Linux behavior:

import Foundation

let s = "Hello\u{200D}World"
print("String: \(s)")
print("Count (graphemes): \(s.count)")
print("UTF-8 count: \(s.utf8.count)")
print("UTF-16 count: \(s.utf16.count)")
print("Unicode scalars count: \(s.unicodeScalars.count)")
print("")

// Check grapheme boundaries
print("Characters (grapheme clusters):")
for (i, char) in s.enumerated() {
     let scalars = char.unicodeScalars.map { $0.value }
     print("  \(i): scalars: \(scalars)")
 }
 print("")

 // Check if "o" and ZWJ form a single grapheme
let utf8 = s.utf8
let idx5 = utf8.index(utf8.startIndex, offsetBy: 5) // After "Hello"
print("UTF-8 index after 5 bytes points to scalar value: \(s.unicodeScalars[idx5].value)")

// Check UTF-16 distance
let utf16 = s.utf16
let utf16Dist = utf16.distance(from: utf16.startIndex, to: idx5)
print("UTF-16 distance to idx5: \(utf16Dist)")

// Now test with NSString
let ns = s as NSString
print("")
print("NSString length: \(ns.length)")

// Test replacement
let range = NSRange(location: 5, length: 1)  // Should be just ZWJ
let result = ns.replacingCharacters(in: range, with: "")
print("After replacing NSRange(5, 1): \(result)")

let range2 = NSRange(location: 4, length: 1)  // Just 'o'
let result2 = ns.replacingCharacters(in: range2, with: "")
print("After replacing NSRange(4, 1): \(result2)")

[SimplyDanny]

Impressive results!

[kapitoshka438]

@SimplyDanny is this ready for merge or anything else you would like to improve?

[kapitoshka438]

@SimplyDanny yet another rebase / resolve conflicts / make register is done. Looking forward to merge this PR.

[SimplyDanny]

Sorry for the late reaction, @kapitoshka438. I have been quite busy with other tasks.

I tried to address your recommendation with #6570. Could that resolve the issue and harmonize behavior?

[kapitoshka438]

Sorry for the late reaction, @kapitoshka438. I have been quite busy with other tasks.

I tried to address your recommendation with #6570. Could that resolve the issue and harmonize behavior?

Hey @SimplyDanny . I've applied your changed locally and can prove that it works and allow to remove the Windows/Linux workaround in InvisibleCharacterRule.swift.
Changed in TestHelpers.swift are still necessary. Windows test run just stucks when running tests.

[SimplyDanny]

Sorry for the late reaction, @kapitoshka438. I have been quite busy with other tasks.
I tried to address your recommendation with #6570. Could that resolve the issue and harmonize behavior?

Hey @SimplyDanny . I've applied your changed locally and can prove that it works and allow to remove the Windows/Linux workaround in InvisibleCharacterRule.swift. Changed in TestHelpers.swift are still necessary. Windows test run just stucks when running tests.

That's good news for the rule implementation part! Obviously, there are more cleanups necessary in TestHelpers. We can do this as a follow-up though.

@kapitoshka438: Would you like to rebase and update your implementation?

[kapitoshka438]

Sorry for the late reaction, @kapitoshka438. I have been quite busy with other tasks.
I tried to address your recommendation with #6570. Could that resolve the issue and harmonize behavior?

Hey @SimplyDanny . I've applied your changed locally and can prove that it works and allow to remove the Windows/Linux workaround in InvisibleCharacterRule.swift. Changed in TestHelpers.swift are still necessary. Windows test run just stucks when running tests.

That's good news for the rule implementation part! Obviously, there are more cleanups necessary in TestHelpers. We can do this as a follow-up though.

@kapitoshka438: Would you like to rebase and update your implementation?

@SimplyDanny done

[SimplyDanny]

Thanks for all the effort @kapitoshka438!

I'll check how TestHelpers can be improved in a similar way to avoid the other workaround.