From 7a129db8af05796413bc41880a018262a6d85ba9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= Date: Tue, 21 Jan 2025 20:28:07 +0100 Subject: [PATCH 1/4] Fix crash when disable command is preceded by unicode character --- CHANGELOG.md | 3 + .../Lint/InvalidSwiftLintCommandRule.swift | 63 ++++++++++++------- 2 files changed, 45 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e950356916..2f292d0632 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,9 @@ * Fix issue referencing the Tests package from another Bazel workspace. [jszumski](https://github.com/jszumski) +* Fix crash when a disable command is preceded by a unicode character. + [SimplyDanny](https://github.com/SimplyDanny) + [#5945](https://github.com/realm/SwiftLint/issues/5945) * Allow severity of `duplicate_imports` rule to be configurable. [SimplyDanny](https://github.com/SimplyDanny) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/InvalidSwiftLintCommandRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/InvalidSwiftLintCommandRule.swift index 52d8146bb0..edde83b683 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/InvalidSwiftLintCommandRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/InvalidSwiftLintCommandRule.swift @@ -1,3 +1,5 @@ +import Foundation + struct InvalidSwiftLintCommandRule: Rule, SourceKitFreeRule { var configuration = SeverityConfiguration(.warning) @@ -13,6 +15,7 @@ struct InvalidSwiftLintCommandRule: Rule, SourceKitFreeRule { Example("// swiftlint:disable:previous unused_import"), Example("// swiftlint:disable:this unused_import"), Example("//swiftlint:disable:this unused_import"), + Example("_ = \"🤵🏼‍♀️\" // swiftlint:disable:this unused_import"), ], triggeringExamples: [ Example("// ↓swiftlint:"), @@ -39,14 +42,13 @@ struct InvalidSwiftLintCommandRule: Rule, SourceKitFreeRule { private func badPrefixViolations(in file: SwiftLintFile) -> [StyleViolation] { (file.commands + file.invalidCommands).compactMap { command in - if let precedingCharacter = command.precedingCharacter(in: file)?.trimmingCharacters(in: .whitespaces) { - if !precedingCharacter.isEmpty, precedingCharacter != "/" { - return styleViolation( - for: command, - in: file, - reason: "swiftlint command should be preceded by whitespace or a comment character" - ) - } + if let precedingCharacter = command.precedingCharacter(in: file)?.unicodeScalars.first, + !CharacterSet.whitespaces.union(CharacterSet(charactersIn: "/")).contains(precedingCharacter) { + return styleViolation( + for: command, + in: file, + reason: "swiftlint command should be preceded by whitespace or a comment character" + ) } return nil } @@ -60,35 +62,54 @@ struct InvalidSwiftLintCommandRule: Rule, SourceKitFreeRule { private func styleViolation(for command: Command, in file: SwiftLintFile, reason: String) -> StyleViolation { let character = command.startingCharacterPosition(in: file) + let characterOffset = character.flatMap { + if let line = command.lineOfCommand(in: file) { + return line.distance(from: line.startIndex, to: $0) + } + return nil + } return StyleViolation( ruleDescription: Self.description, severity: configuration.severity, - location: Location(file: file.path, line: command.line, character: character), + location: Location(file: file.path, line: command.line, character: characterOffset), reason: reason ) } } private extension Command { - func startingCharacterPosition(in file: SwiftLintFile) -> Int? { - var position = character - if line > 0, line <= file.lines.count { - let line = file.lines[line - 1].content - if let commandIndex = line.range(of: "swiftlint:")?.lowerBound { - position = line.distance(from: line.startIndex, to: commandIndex) + 1 - } + func lineOfCommand(in file: SwiftLintFile) -> String? { + guard line > 0, line <= file.lines.count else { + return nil } - return position + return file.lines[line - 1].content } - func precedingCharacter(in file: SwiftLintFile) -> String? { - if let startingCharacterPosition = startingCharacterPosition(in: file), startingCharacterPosition > 2 { - let line = file.lines[line - 1].content - return line.substring(from: startingCharacterPosition - 2, length: 1) + func startingCharacterPosition(in file: SwiftLintFile) -> String.Index? { + guard let line = lineOfCommand(in: file), line.isNotEmpty else { + return nil + } + if let commandIndex = line.range(of: "swiftlint:")?.lowerBound { + let distance = line.distance(from: line.startIndex, to: commandIndex) + return line.index(line.startIndex, offsetBy: distance + 1) + } + if let character { + return line.index(line.startIndex, offsetBy: character) } return nil } + func precedingCharacter(in file: SwiftLintFile) -> Character? { + guard let startingCharacterPosition = startingCharacterPosition(in: file), + let line = lineOfCommand(in: file) else { + return nil + } + guard line.distance(from: line.startIndex, to: startingCharacterPosition) > 2 else { + return nil + } + return line[line.index(startingCharacterPosition, offsetBy: -2)...].first + } + func invalidReason() -> String? { if action == .invalid { return "swiftlint command does not have a valid action" From f486bb36c86206fb9760658adb2b1258726623f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= Date: Fri, 24 Jan 2025 07:58:06 +0100 Subject: [PATCH 2/4] Record start positions of commands --- CHANGELOG.md | 1 + .../Lint/InvalidSwiftLintCommandRule.swift | 59 +++++-------------- .../Extensions/String+SwiftLint.swift | 13 ++++ Source/SwiftLintCore/Models/Command.swift | 8 +-- .../Visitors/CommandVisitor.swift | 43 +++++++------- Tests/FrameworkTests/CommandTests.swift | 42 ++++++------- Tests/FrameworkTests/RegionTests.swift | 53 +++++++++-------- .../FrameworkTests/StringExtensionTests.swift | 16 +++++ 8 files changed, 118 insertions(+), 117 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f292d0632..8f9719566f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ * Fix issue referencing the Tests package from another Bazel workspace. [jszumski](https://github.com/jszumski) + * Fix crash when a disable command is preceded by a unicode character. [SimplyDanny](https://github.com/SimplyDanny) [#5945](https://github.com/realm/SwiftLint/issues/5945) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/InvalidSwiftLintCommandRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/InvalidSwiftLintCommandRule.swift index edde83b683..b06d9d0ed2 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/InvalidSwiftLintCommandRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/InvalidSwiftLintCommandRule.swift @@ -15,7 +15,8 @@ struct InvalidSwiftLintCommandRule: Rule, SourceKitFreeRule { Example("// swiftlint:disable:previous unused_import"), Example("// swiftlint:disable:this unused_import"), Example("//swiftlint:disable:this unused_import"), - Example("_ = \"🤵🏼‍♀️\" // swiftlint:disable:this unused_import"), + Example("_ = \"🤵🏼‍♀️\" // swiftlint:disable:this unused_import", excludeFromDocumentation: true), + Example("_ = \"🤵🏼‍♀️ 🤵🏼‍♀️\" // swiftlint:disable:this unused_import", excludeFromDocumentation: true), ], triggeringExamples: [ Example("// ↓swiftlint:"), @@ -33,6 +34,7 @@ struct InvalidSwiftLintCommandRule: Rule, SourceKitFreeRule { Example("// ↓swiftlint:enable: "), Example("// ↓swiftlint:disable: unused_import"), Example("// s↓swiftlint:disable unused_import"), + Example("// 🤵🏼‍♀️swiftlint:disable unused_import", excludeFromDocumentation: true), ].skipWrappingInCommentTests() ) @@ -42,15 +44,13 @@ struct InvalidSwiftLintCommandRule: Rule, SourceKitFreeRule { private func badPrefixViolations(in file: SwiftLintFile) -> [StyleViolation] { (file.commands + file.invalidCommands).compactMap { command in - if let precedingCharacter = command.precedingCharacter(in: file)?.unicodeScalars.first, - !CharacterSet.whitespaces.union(CharacterSet(charactersIn: "/")).contains(precedingCharacter) { - return styleViolation( + command.isPrecededByInvalidCharacter(in: file) + ? styleViolation( for: command, in: file, reason: "swiftlint command should be preceded by whitespace or a comment character" ) - } - return nil + : nil } } @@ -61,53 +61,26 @@ struct InvalidSwiftLintCommandRule: Rule, SourceKitFreeRule { } private func styleViolation(for command: Command, in file: SwiftLintFile, reason: String) -> StyleViolation { - let character = command.startingCharacterPosition(in: file) - let characterOffset = character.flatMap { - if let line = command.lineOfCommand(in: file) { - return line.distance(from: line.startIndex, to: $0) - } - return nil - } - return StyleViolation( + StyleViolation( ruleDescription: Self.description, severity: configuration.severity, - location: Location(file: file.path, line: command.line, character: characterOffset), + location: Location(file: file.path, line: command.line, character: command.character), reason: reason ) } } private extension Command { - func lineOfCommand(in file: SwiftLintFile) -> String? { - guard line > 0, line <= file.lines.count else { - return nil - } - return file.lines[line - 1].content - } - - func startingCharacterPosition(in file: SwiftLintFile) -> String.Index? { - guard let line = lineOfCommand(in: file), line.isNotEmpty else { - return nil - } - if let commandIndex = line.range(of: "swiftlint:")?.lowerBound { - let distance = line.distance(from: line.startIndex, to: commandIndex) - return line.index(line.startIndex, offsetBy: distance + 1) - } - if let character { - return line.index(line.startIndex, offsetBy: character) - } - return nil - } - - func precedingCharacter(in file: SwiftLintFile) -> Character? { - guard let startingCharacterPosition = startingCharacterPosition(in: file), - let line = lineOfCommand(in: file) else { - return nil + func isPrecededByInvalidCharacter(in file: SwiftLintFile) -> Bool { + guard line > 0, let character, character > 1, line <= file.lines.count else { + return false } - guard line.distance(from: line.startIndex, to: startingCharacterPosition) > 2 else { - return nil + let line = file.lines[line - 1].content + guard line.count > character, + let char = line[line.index(line.startIndex, offsetBy: character - 2)].unicodeScalars.first else { + return false } - return line[line.index(startingCharacterPosition, offsetBy: -2)...].first + return !CharacterSet.whitespaces.union(CharacterSet(charactersIn: "/")).contains(char) } func invalidReason() -> String? { diff --git a/Source/SwiftLintCore/Extensions/String+SwiftLint.swift b/Source/SwiftLintCore/Extensions/String+SwiftLint.swift index 49839438b7..41c19f8bff 100644 --- a/Source/SwiftLintCore/Extensions/String+SwiftLint.swift +++ b/Source/SwiftLintCore/Extensions/String+SwiftLint.swift @@ -128,4 +128,17 @@ public extension String { func linesPrefixed(with prefix: Self) -> Self { split(separator: "\n").joined(separator: "\n\(prefix)") } + + func characterPosition(of utf8Offset: Int) -> Int? { + guard utf8Offset != 0 else { + return 0 + } + guard utf8Offset > 0, utf8Offset < lengthOfBytes(using: .utf8) else { + return nil + } + for (offset, index) in indices.enumerated() where self[...index].lengthOfBytes(using: .utf8) == utf8Offset { + return offset + 1 + } + return nil + } } diff --git a/Source/SwiftLintCore/Models/Command.swift b/Source/SwiftLintCore/Models/Command.swift index 9ebea5b52d..28e41b67e5 100644 --- a/Source/SwiftLintCore/Models/Command.swift +++ b/Source/SwiftLintCore/Models/Command.swift @@ -51,7 +51,7 @@ public struct Command: Equatable { public let ruleIdentifiers: Set /// The line in the source file where this command is defined. public let line: Int - /// The character offset within the line in the source file where this command is defined. + /// The character offset within the line in the source file where this command starts. public let character: Int? /// This command's modifier, if any. public let modifier: Modifier? @@ -63,8 +63,7 @@ public struct Command: Equatable { /// - parameter action: This command's action. /// - parameter ruleIdentifiers: The identifiers for the rules associated with this command. /// - parameter line: The line in the source file where this command is defined. - /// - parameter character: The character offset within the line in the source file where this command is - /// defined. + /// - parameter character: The character offset within the line in the source file where this command starts. /// - parameter modifier: This command's modifier, if any. /// - parameter trailingComment: The comment following this command's `-` delimiter, if any. public init(action: Action, @@ -85,8 +84,7 @@ public struct Command: Equatable { /// /// - parameter actionString: The string in the command's definition describing its action. /// - parameter line: The line in the source file where this command is defined. - /// - parameter character: The character offset within the line in the source file where this command is - /// defined. + /// - parameter character: The character offset within the line in the source file where this command starts. public init(actionString: String, line: Int, character: Int) { let scanner = Scanner(string: actionString) _ = scanner.scanString("swiftlint:") diff --git a/Source/SwiftLintCore/Visitors/CommandVisitor.swift b/Source/SwiftLintCore/Visitors/CommandVisitor.swift index bd383d8782..58128292da 100644 --- a/Source/SwiftLintCore/Visitors/CommandVisitor.swift +++ b/Source/SwiftLintCore/Visitors/CommandVisitor.swift @@ -13,37 +13,34 @@ final class CommandVisitor: SyntaxVisitor { } override func visitPost(_ node: TokenSyntax) { - let leadingCommands = node.leadingTrivia.commands(offset: node.position, - locationConverter: locationConverter) - let trailingCommands = node.trailingTrivia.commands(offset: node.endPositionBeforeTrailingTrivia, - locationConverter: locationConverter) - self.commands.append(contentsOf: leadingCommands + trailingCommands) + collectCommands(in: node.leadingTrivia, offset: node.position) + collectCommands(in: node.trailingTrivia, offset: node.endPositionBeforeTrailingTrivia) } -} - -// MARK: - Private Helpers -private extension Trivia { - func commands(offset: AbsolutePosition, locationConverter: SourceLocationConverter) -> [Command] { - var triviaOffset = SourceLength.zero - var results: [Command] = [] - for trivia in self { - triviaOffset += trivia.sourceLength - switch trivia { + private func collectCommands(in trivia: Trivia, offset: AbsolutePosition) { + var position = offset + for piece in trivia { + switch piece { case .lineComment(let comment): - guard let lower = comment.range(of: "swiftlint:")?.lowerBound else { + guard let lower = comment.range(of: "swiftlint:")?.lowerBound.samePosition(in: comment.utf8) else { break } - - let actionString = String(comment[lower...]) - let end = locationConverter.location(for: offset + triviaOffset) - let command = Command(actionString: actionString, line: end.line, character: end.column) - results.append(command) + let offset = comment.utf8.distance(from: comment.utf8.startIndex, to: lower) + let location = locationConverter.location(for: position.advanced(by: offset)) + let line = locationConverter.sourceLines[location.line - 1] + guard let character = line.characterPosition(of: location.column) else { + break + } + let command = Command( + actionString: String(comment[lower...]), + line: location.line, + character: character + ) + commands.append(command) default: break } + position += piece.sourceLength } - - return results } } diff --git a/Tests/FrameworkTests/CommandTests.swift b/Tests/FrameworkTests/CommandTests.swift index 1859c369d1..cbea58e8a5 100644 --- a/Tests/FrameworkTests/CommandTests.swift +++ b/Tests/FrameworkTests/CommandTests.swift @@ -10,7 +10,7 @@ private extension Command { let nsString = string.bridge() guard nsString.length > 7 else { return nil } let subString = nsString.substring(with: NSRange(location: 3, length: nsString.length - 4)) - self.init(actionString: subString, line: 1, character: nsString.length) + self.init(actionString: subString, line: 1, character: 4) } } @@ -30,7 +30,7 @@ final class CommandTests: SwiftLintTestCase { func testDisable() { let input = "// swiftlint:disable rule_id\n" let file = SwiftLintFile(contents: input) - let expected = Command(action: .disable, ruleIdentifiers: ["rule_id"], line: 1, character: 29, modifier: nil) + let expected = Command(action: .disable, ruleIdentifiers: ["rule_id"], line: 1, character: 4, modifier: nil) XCTAssertEqual(file.commands(), [expected]) XCTAssertEqual(Command(string: input), expected) } @@ -38,7 +38,7 @@ final class CommandTests: SwiftLintTestCase { func testDisablePrevious() { let input = "// swiftlint:disable:previous rule_id\n" let file = SwiftLintFile(contents: input) - let expected = Command(action: .disable, ruleIdentifiers: ["rule_id"], line: 1, character: 38, + let expected = Command(action: .disable, ruleIdentifiers: ["rule_id"], line: 1, character: 4, modifier: .previous) XCTAssertEqual(file.commands(), expected.expand()) XCTAssertEqual(Command(string: input), expected) @@ -47,7 +47,7 @@ final class CommandTests: SwiftLintTestCase { func testDisableThis() { let input = "// swiftlint:disable:this rule_id\n" let file = SwiftLintFile(contents: input) - let expected = Command(action: .disable, ruleIdentifiers: ["rule_id"], line: 1, character: 34, modifier: .this) + let expected = Command(action: .disable, ruleIdentifiers: ["rule_id"], line: 1, character: 4, modifier: .this) XCTAssertEqual(file.commands(), expected.expand()) XCTAssertEqual(Command(string: input), expected) } @@ -55,7 +55,7 @@ final class CommandTests: SwiftLintTestCase { func testDisableNext() { let input = "// swiftlint:disable:next rule_id\n" let file = SwiftLintFile(contents: input) - let expected = Command(action: .disable, ruleIdentifiers: ["rule_id"], line: 1, character: 34, modifier: .next) + let expected = Command(action: .disable, ruleIdentifiers: ["rule_id"], line: 1, character: 4, modifier: .next) XCTAssertEqual(file.commands(), expected.expand()) XCTAssertEqual(Command(string: input), expected) } @@ -63,7 +63,7 @@ final class CommandTests: SwiftLintTestCase { func testEnable() { let input = "// swiftlint:enable rule_id\n" let file = SwiftLintFile(contents: input) - let expected = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 28, modifier: nil) + let expected = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 4, modifier: nil) XCTAssertEqual(file.commands(), [expected]) XCTAssertEqual(Command(string: input), expected) } @@ -71,7 +71,7 @@ final class CommandTests: SwiftLintTestCase { func testEnablePrevious() { let input = "// swiftlint:enable:previous rule_id\n" let file = SwiftLintFile(contents: input) - let expected = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 37, + let expected = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 4, modifier: .previous) XCTAssertEqual(file.commands(), expected.expand()) XCTAssertEqual(Command(string: input), expected) @@ -80,7 +80,7 @@ final class CommandTests: SwiftLintTestCase { func testEnableThis() { let input = "// swiftlint:enable:this rule_id\n" let file = SwiftLintFile(contents: input) - let expected = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 33, modifier: .this) + let expected = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 4, modifier: .this) XCTAssertEqual(file.commands(), expected.expand()) XCTAssertEqual(Command(string: input), expected) } @@ -88,7 +88,7 @@ final class CommandTests: SwiftLintTestCase { func testEnableNext() { let input = "// swiftlint:enable:next rule_id\n" let file = SwiftLintFile(contents: input) - let expected = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 33, modifier: .next) + let expected = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 4, modifier: .next) XCTAssertEqual(file.commands(), expected.expand()) XCTAssertEqual(Command(string: input), expected) } @@ -96,7 +96,7 @@ final class CommandTests: SwiftLintTestCase { func testTrailingComment() { let input = "// swiftlint:enable:next rule_id - Comment\n" let file = SwiftLintFile(contents: input) - let expected = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 43, modifier: .next, + let expected = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 4, modifier: .next, trailingComment: "Comment") XCTAssertEqual(file.commands(), expected.expand()) XCTAssertEqual(Command(string: input), expected) @@ -105,7 +105,7 @@ final class CommandTests: SwiftLintTestCase { func testTrailingCommentWithUrl() { let input = "// swiftlint:enable:next rule_id - Comment with URL https://github.com/realm/SwiftLint\n" let file = SwiftLintFile(contents: input) - let expected = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 87, modifier: .next, + let expected = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 4, modifier: .next, trailingComment: "Comment with URL https://github.com/realm/SwiftLint") XCTAssertEqual(file.commands(), expected.expand()) XCTAssertEqual(Command(string: input), expected) @@ -114,7 +114,7 @@ final class CommandTests: SwiftLintTestCase { func testTrailingCommentUrlOnly() { let input = "// swiftlint:enable:next rule_id - https://github.com/realm/SwiftLint\n" let file = SwiftLintFile(contents: input) - let expected = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 70, modifier: .next, + let expected = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 4, modifier: .next, trailingComment: "https://github.com/realm/SwiftLint") XCTAssertEqual(file.commands(), expected.expand()) XCTAssertEqual(Command(string: input), expected) @@ -146,7 +146,7 @@ final class CommandTests: SwiftLintTestCase { func testExpandPreviousCommand() { do { - let command = Command(action: .disable, ruleIdentifiers: ["rule_id"], line: 1, character: 38, + let command = Command(action: .disable, ruleIdentifiers: ["rule_id"], line: 1, character: 48, modifier: .previous) let expanded = [ Command(action: .disable, ruleIdentifiers: ["rule_id"], line: 0, character: nil), @@ -155,7 +155,7 @@ final class CommandTests: SwiftLintTestCase { XCTAssertEqual(command.expand(), expanded) } do { - let command = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 38, + let command = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 48, modifier: .previous) let expanded = [ Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 0, character: nil), @@ -164,7 +164,7 @@ final class CommandTests: SwiftLintTestCase { XCTAssertEqual(command.expand(), expanded) } do { - let command = Command(action: .enable, ruleIdentifiers: ["1", "2"], line: 1, character: 38, + let command = Command(action: .enable, ruleIdentifiers: ["1", "2"], line: 1, character: 48, modifier: .previous) let expanded = [ Command(action: .enable, ruleIdentifiers: ["1", "2"], line: 0, character: nil), @@ -176,7 +176,7 @@ final class CommandTests: SwiftLintTestCase { func testExpandThisCommand() { do { - let command = Command(action: .disable, ruleIdentifiers: ["rule_id"], line: 1, character: 38, + let command = Command(action: .disable, ruleIdentifiers: ["rule_id"], line: 1, character: 48, modifier: .this) let expanded = [ Command(action: .disable, ruleIdentifiers: ["rule_id"], line: 1, character: nil), @@ -185,7 +185,7 @@ final class CommandTests: SwiftLintTestCase { XCTAssertEqual(command.expand(), expanded) } do { - let command = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 38, + let command = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 48, modifier: .this) let expanded = [ Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: nil), @@ -194,7 +194,7 @@ final class CommandTests: SwiftLintTestCase { XCTAssertEqual(command.expand(), expanded) } do { - let command = Command(action: .enable, ruleIdentifiers: ["1", "2"], line: 1, character: 38, + let command = Command(action: .enable, ruleIdentifiers: ["1", "2"], line: 1, character: 48, modifier: .this) let expanded = [ Command(action: .enable, ruleIdentifiers: ["1", "2"], line: 1, character: nil), @@ -206,7 +206,7 @@ final class CommandTests: SwiftLintTestCase { func testExpandNextCommand() { do { - let command = Command(action: .disable, ruleIdentifiers: ["rule_id"], line: 1, character: 38, + let command = Command(action: .disable, ruleIdentifiers: ["rule_id"], line: 1, character: 48, modifier: .next) let expanded = [ Command(action: .disable, ruleIdentifiers: ["rule_id"], line: 2, character: nil), @@ -215,7 +215,7 @@ final class CommandTests: SwiftLintTestCase { XCTAssertEqual(command.expand(), expanded) } do { - let command = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 38, + let command = Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 1, character: 48, modifier: .next) let expanded = [ Command(action: .enable, ruleIdentifiers: ["rule_id"], line: 2, character: nil), @@ -224,7 +224,7 @@ final class CommandTests: SwiftLintTestCase { XCTAssertEqual(command.expand(), expanded) } do { - let command = Command(action: .enable, ruleIdentifiers: ["1", "2"], line: 1, character: 38, + let command = Command(action: .enable, ruleIdentifiers: ["1", "2"], line: 1, character: 48, modifier: .next) let expanded = [ Command(action: .enable, ruleIdentifiers: ["1", "2"], line: 2, character: nil), diff --git a/Tests/FrameworkTests/RegionTests.swift b/Tests/FrameworkTests/RegionTests.swift index 3935f583c7..ee9be3ec9d 100644 --- a/Tests/FrameworkTests/RegionTests.swift +++ b/Tests/FrameworkTests/RegionTests.swift @@ -18,14 +18,14 @@ final class RegionTests: SwiftLintTestCase { // disable do { let file = SwiftLintFile(contents: "// swiftlint:disable rule_id\n") - let start = Location(file: nil, line: 1, character: 29) + let start = Location(file: nil, line: 1, character: 4) let end = Location(file: nil, line: .max, character: .max) XCTAssertEqual(file.regions(), [Region(start: start, end: end, disabledRuleIdentifiers: ["rule_id"])]) } // enable do { let file = SwiftLintFile(contents: "// swiftlint:enable rule_id\n") - let start = Location(file: nil, line: 1, character: 28) + let start = Location(file: nil, line: 1, character: 4) let end = Location(file: nil, line: .max, character: .max) XCTAssertEqual(file.regions(), [Region(start: start, end: end, disabledRuleIdentifiers: [])]) } @@ -36,10 +36,10 @@ final class RegionTests: SwiftLintTestCase { do { let file = SwiftLintFile(contents: "// swiftlint:disable rule_id\n// swiftlint:enable rule_id\n") XCTAssertEqual(file.regions(), [ - Region(start: Location(file: nil, line: 1, character: 29), - end: Location(file: nil, line: 2, character: 27), + Region(start: Location(file: nil, line: 1, character: 4), + end: Location(file: nil, line: 2, character: 3), disabledRuleIdentifiers: ["rule_id"]), - Region(start: Location(file: nil, line: 2, character: 28), + Region(start: Location(file: nil, line: 2, character: 4), end: Location(file: nil, line: .max, character: .max), disabledRuleIdentifiers: []), ]) @@ -48,10 +48,10 @@ final class RegionTests: SwiftLintTestCase { do { let file = SwiftLintFile(contents: "// swiftlint:enable rule_id\n// swiftlint:disable rule_id\n") XCTAssertEqual(file.regions(), [ - Region(start: Location(file: nil, line: 1, character: 28), - end: Location(file: nil, line: 2, character: 28), + Region(start: Location(file: nil, line: 1, character: 4), + end: Location(file: nil, line: 2, character: 3), disabledRuleIdentifiers: []), - Region(start: Location(file: nil, line: 2, character: 29), + Region(start: Location(file: nil, line: 2, character: 4), end: Location(file: nil, line: .max, character: .max), disabledRuleIdentifiers: ["rule_id"]), ]) @@ -73,29 +73,32 @@ final class RegionTests: SwiftLintTestCase { } func testSeveralRegionsFromSeveralCommands() { - let file = SwiftLintFile(contents: "// swiftlint:disable 1\n" + - "// swiftlint:disable 2\n" + - "// swiftlint:disable 3\n" + - "// swiftlint:enable 1\n" + - "// swiftlint:enable 2\n" + - "// swiftlint:enable 3\n") + let file = SwiftLintFile(contents: """ + // swiftlint:disable 1 + // swiftlint:disable 2 + // swiftlint:disable 3 + // swiftlint:enable 1 + // swiftlint:enable 2 + // swiftlint:enable 3 + """ + ) XCTAssertEqual(file.regions(), [ - Region(start: Location(file: nil, line: 1, character: 23), - end: Location(file: nil, line: 2, character: 22), + Region(start: Location(file: nil, line: 1, character: 4), + end: Location(file: nil, line: 2, character: 3), disabledRuleIdentifiers: ["1"]), - Region(start: Location(file: nil, line: 2, character: 23), - end: Location(file: nil, line: 3, character: 22), + Region(start: Location(file: nil, line: 2, character: 4), + end: Location(file: nil, line: 3, character: 3), disabledRuleIdentifiers: ["1", "2"]), - Region(start: Location(file: nil, line: 3, character: 23), - end: Location(file: nil, line: 4, character: 21), + Region(start: Location(file: nil, line: 3, character: 4), + end: Location(file: nil, line: 4, character: 3), disabledRuleIdentifiers: ["1", "2", "3"]), - Region(start: Location(file: nil, line: 4, character: 22), - end: Location(file: nil, line: 5, character: 21), + Region(start: Location(file: nil, line: 4, character: 4), + end: Location(file: nil, line: 5, character: 3), disabledRuleIdentifiers: ["2", "3"]), - Region(start: Location(file: nil, line: 5, character: 22), - end: Location(file: nil, line: 6, character: 21), + Region(start: Location(file: nil, line: 5, character: 4), + end: Location(file: nil, line: 6, character: 3), disabledRuleIdentifiers: ["3"]), - Region(start: Location(file: nil, line: 6, character: 22), + Region(start: Location(file: nil, line: 6, character: 4), end: Location(file: nil, line: .max, character: .max), disabledRuleIdentifiers: []), ]) diff --git a/Tests/FrameworkTests/StringExtensionTests.swift b/Tests/FrameworkTests/StringExtensionTests.swift index b253f74a8c..d7b394fd60 100644 --- a/Tests/FrameworkTests/StringExtensionTests.swift +++ b/Tests/FrameworkTests/StringExtensionTests.swift @@ -28,4 +28,20 @@ final class StringExtensionTests: SwiftLintTestCase { """ ) } + + func testCharacterPosition() { + XCTAssertNil("string".characterPosition(of: -1)) + XCTAssertEqual("string".characterPosition(of: 0), 0) + XCTAssertEqual("string".characterPosition(of: 1), 1) + XCTAssertNil("string".characterPosition(of: 6)) + XCTAssertNil("string".characterPosition(of: 7)) + + XCTAssertEqual("s🤵🏼‍♀️s".characterPosition(of: 0), 0) + XCTAssertEqual("s🤵🏼‍♀️s".characterPosition(of: 1), 1) + for bytes in 2...17 { + XCTAssertNil("s🤵🏼‍♀️s".characterPosition(of: bytes)) + } + XCTAssertEqual("s🤵🏼‍♀️s".characterPosition(of: 18), 2) + XCTAssertNil("s🤵🏼‍♀️s".characterPosition(of: 19)) + } } From 7ad0dfcdec5d77e948be9f97bd13a3019b8b233d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= Date: Mon, 27 Jan 2025 21:32:26 +0100 Subject: [PATCH 3/4] Record range of command to know exactly where a new region starts --- .../Lint/BlanketDisableCommandRule.swift | 3 +- .../Lint/InvalidSwiftLintCommandRule.swift | 4 +- .../Extensions/SwiftLintFile+Regex.swift | 6 +- Source/SwiftLintCore/Models/Command.swift | 26 +++--- .../Visitors/CommandVisitor.swift | 2 +- Tests/FrameworkTests/CommandTests.swift | 85 ++++++++++--------- Tests/FrameworkTests/RegionTests.swift | 38 ++++----- 7 files changed, 86 insertions(+), 78 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/BlanketDisableCommandRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/BlanketDisableCommandRule.swift index 2a576740f7..f2a1b3e8e7 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/BlanketDisableCommandRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/BlanketDisableCommandRule.swift @@ -38,6 +38,7 @@ struct BlanketDisableCommandRule: Rule, SourceKitFreeRule { Example(""" // swiftlint:enable ↓unused_import """), + Example("// swiftlint:disable all"), ].skipWrappingInCommentTests().skipDisableCommandTests() ) @@ -189,7 +190,7 @@ struct BlanketDisableCommandRule: Rule, SourceKitFreeRule { private extension Command { func location(of ruleIdentifier: String, in file: SwiftLintFile) -> Location { - var location = character + var location = range?.upperBound if line > 0, line <= file.lines.count { let line = file.lines[line - 1].content if let ruleIdentifierIndex = line.range(of: ruleIdentifier)?.lowerBound { diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/InvalidSwiftLintCommandRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/InvalidSwiftLintCommandRule.swift index b06d9d0ed2..a2e248420f 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/InvalidSwiftLintCommandRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/InvalidSwiftLintCommandRule.swift @@ -64,7 +64,7 @@ struct InvalidSwiftLintCommandRule: Rule, SourceKitFreeRule { StyleViolation( ruleDescription: Self.description, severity: configuration.severity, - location: Location(file: file.path, line: command.line, character: command.character), + location: Location(file: file.path, line: command.line, character: command.range?.lowerBound), reason: reason ) } @@ -72,7 +72,7 @@ struct InvalidSwiftLintCommandRule: Rule, SourceKitFreeRule { private extension Command { func isPrecededByInvalidCharacter(in file: SwiftLintFile) -> Bool { - guard line > 0, let character, character > 1, line <= file.lines.count else { + guard line > 0, let character = range?.lowerBound, character > 1, line <= file.lines.count else { return false } let line = file.lines[line - 1].content diff --git a/Source/SwiftLintCore/Extensions/SwiftLintFile+Regex.swift b/Source/SwiftLintCore/Extensions/SwiftLintFile+Regex.swift index c8fe43d36d..1f4ce74a43 100644 --- a/Source/SwiftLintCore/Extensions/SwiftLintFile+Regex.swift +++ b/Source/SwiftLintCore/Extensions/SwiftLintFile+Regex.swift @@ -36,7 +36,7 @@ extension SwiftLintFile { break } - let start = Location(file: path, line: command.line, character: command.character) + let start = Location(file: path, line: command.line, character: command.range?.upperBound) let end = endOf(next: nextCommand) guard start < end else { continue } var didSetRegion = false @@ -67,7 +67,7 @@ extension SwiftLintFile { let rangeEnd = Location(file: self, characterOffset: NSMaxRange(range)) return commands .filter { command in - let commandLocation = Location(file: path, line: command.line, character: command.character) + let commandLocation = Location(file: path, line: command.line, character: command.range?.upperBound) return rangeStart <= commandLocation && commandLocation <= rangeEnd } .flatMap { $0.expand() } @@ -79,7 +79,7 @@ extension SwiftLintFile { } let nextLine: Int let nextCharacter: Int? - if let nextCommandCharacter = nextCommand.character { + if let nextCommandCharacter = nextCommand.range?.upperBound { nextLine = nextCommand.line if nextCommandCharacter > 0 { nextCharacter = nextCommandCharacter - 1 diff --git a/Source/SwiftLintCore/Models/Command.swift b/Source/SwiftLintCore/Models/Command.swift index 28e41b67e5..c6659522fe 100644 --- a/Source/SwiftLintCore/Models/Command.swift +++ b/Source/SwiftLintCore/Models/Command.swift @@ -51,8 +51,8 @@ public struct Command: Equatable { public let ruleIdentifiers: Set /// The line in the source file where this command is defined. public let line: Int - /// The character offset within the line in the source file where this command starts. - public let character: Int? + /// The range of the command in the line (0-based). + public let range: Range? /// This command's modifier, if any. public let modifier: Modifier? /// The comment following this command's `-` delimiter, if any. @@ -63,19 +63,19 @@ public struct Command: Equatable { /// - parameter action: This command's action. /// - parameter ruleIdentifiers: The identifiers for the rules associated with this command. /// - parameter line: The line in the source file where this command is defined. - /// - parameter character: The character offset within the line in the source file where this command starts. + /// - parameter range: The range of the command in the line (0-based). /// - parameter modifier: This command's modifier, if any. /// - parameter trailingComment: The comment following this command's `-` delimiter, if any. public init(action: Action, ruleIdentifiers: Set = [], line: Int = 0, - character: Int? = nil, + range: Range? = nil, modifier: Modifier? = nil, trailingComment: String? = nil) { self.action = action self.ruleIdentifiers = ruleIdentifiers self.line = line - self.character = character + self.range = range self.modifier = modifier self.trailingComment = trailingComment } @@ -84,20 +84,20 @@ public struct Command: Equatable { /// /// - parameter actionString: The string in the command's definition describing its action. /// - parameter line: The line in the source file where this command is defined. - /// - parameter character: The character offset within the line in the source file where this command starts. - public init(actionString: String, line: Int, character: Int) { + /// - parameter range: The range of the command in the line (0-based). + public init(actionString: String, line: Int, range: Range) { let scanner = Scanner(string: actionString) _ = scanner.scanString("swiftlint:") // (enable|disable)(:previous|:this|:next) guard let actionAndModifierString = scanner.scanUpToString(" ") else { - self.init(action: .invalid, line: line, character: character) + self.init(action: .invalid, line: line, range: range) return } let actionAndModifierScanner = Scanner(string: actionAndModifierString) guard let actionString = actionAndModifierScanner.scanUpToString(":"), let action = Action(rawValue: actionString) else { - self.init(action: .invalid, line: line, character: character) + self.init(action: .invalid, line: line, range: range) return } @@ -135,7 +135,7 @@ public struct Command: Equatable { action: action, ruleIdentifiers: ruleIdentifiers, line: line, - character: character, + range: range, modifier: modifier, trailingComment: trailingComment ) @@ -153,17 +153,17 @@ public struct Command: Equatable { case .previous: return [ Self(action: action, ruleIdentifiers: ruleIdentifiers, line: line - 1), - Self(action: action.inverse(), ruleIdentifiers: ruleIdentifiers, line: line - 1, character: Int.max), + Self(action: action.inverse(), ruleIdentifiers: ruleIdentifiers, line: line - 1, range: 0.. 7 else { return nil } let subString = nsString.substring(with: NSRange(location: 3, length: nsString.length - 4)) - self.init(actionString: subString, line: 1, character: 4) + self.init(actionString: subString, line: 1, range: 4.. Date: Tue, 28 Jan 2025 15:30:53 +0100 Subject: [PATCH 4/4] Rename `actionString` to `commandString` --- Source/SwiftLintCore/Models/Command.swift | 10 +++++----- Source/SwiftLintCore/Visitors/CommandVisitor.swift | 2 +- Tests/FrameworkTests/CommandTests.swift | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Source/SwiftLintCore/Models/Command.swift b/Source/SwiftLintCore/Models/Command.swift index c6659522fe..09fb2db0bf 100644 --- a/Source/SwiftLintCore/Models/Command.swift +++ b/Source/SwiftLintCore/Models/Command.swift @@ -82,11 +82,11 @@ public struct Command: Equatable { /// Creates a command based on the specified parameters. /// - /// - parameter actionString: The string in the command's definition describing its action. - /// - parameter line: The line in the source file where this command is defined. - /// - parameter range: The range of the command in the line (0-based). - public init(actionString: String, line: Int, range: Range) { - let scanner = Scanner(string: actionString) + /// - parameter commandString: The whole command string as found in the code. + /// - parameter line: The line in the source file where this command is defined. + /// - parameter range: The range of the command in the line (0-based). + public init(commandString: String, line: Int, range: Range) { + let scanner = Scanner(string: commandString) _ = scanner.scanString("swiftlint:") // (enable|disable)(:previous|:this|:next) guard let actionAndModifierString = scanner.scanUpToString(" ") else { diff --git a/Source/SwiftLintCore/Visitors/CommandVisitor.swift b/Source/SwiftLintCore/Visitors/CommandVisitor.swift index 98c2251b4c..530fdf4923 100644 --- a/Source/SwiftLintCore/Visitors/CommandVisitor.swift +++ b/Source/SwiftLintCore/Visitors/CommandVisitor.swift @@ -32,7 +32,7 @@ final class CommandVisitor: SyntaxVisitor { break } let command = Command( - actionString: String(comment[lower...]), + commandString: String(comment[lower...]), line: location.line, range: character..<(character + piece.sourceLength.utf8Length - offset) ) diff --git a/Tests/FrameworkTests/CommandTests.swift b/Tests/FrameworkTests/CommandTests.swift index 5141987a46..964641e72b 100644 --- a/Tests/FrameworkTests/CommandTests.swift +++ b/Tests/FrameworkTests/CommandTests.swift @@ -10,7 +10,7 @@ private extension Command { let nsString = string.bridge() guard nsString.length > 7 else { return nil } let subString = nsString.substring(with: NSRange(location: 3, length: nsString.length - 4)) - self.init(actionString: subString, line: 1, range: 4..