Skip to content

Commit

Permalink
Merge pull request #3416 from noahsark769/noahsark769/comment-spacing…
Browse files Browse the repository at this point in the history
…-rule-updated

Add CommentSpacingRule (#3233)
  • Loading branch information
noahsark769 authored Nov 11, 2020
2 parents eb0c1b2 + 6d4487c commit 2e26e9c
Show file tree
Hide file tree
Showing 14 changed files with 179 additions and 16 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@
`unused_import` rule.
[Keith Smiley](https://github.com/keith)

* Add `comment_spacing` rule.
[Noah Gilmore](https://github.com/noahsark769)
[#3233](https://github.com/realm/SwiftLint/issues/3233)

#### Bug Fixes

* None.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ extension SwiftLintFile {
return tokens.map { $0.kinds }
}

//Added by S2dent
/**
This function returns only matches that are not contained in a syntax kind
specified.
Expand Down
1 change: 1 addition & 0 deletions Source/SwiftLintFramework/Models/PrimaryRuleList.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public let primaryRuleList = RuleList(rules: [
CollectionAlignmentRule.self,
ColonRule.self,
CommaRule.self,
CommentSpacingRule.self,
CompilerProtocolInitRule.self,
ComputedAccessorsOrderRule.self,
ConditionalReturnsOnNewlineRule.self,
Expand Down
145 changes: 145 additions & 0 deletions Source/SwiftLintFramework/Rules/Lint/CommentSpacingRule.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
import Foundation
import SourceKittenFramework

public struct CommentSpacingRule: ConfigurationProviderRule,
SubstitutionCorrectableRule,
AutomaticTestableRule {
public var configuration = SeverityConfiguration(.warning)

public init() {}

public static let description = RuleDescription(
identifier: "comment_spacing",
name: "Comment Spacing",
description: "Prefer at least one space after slashes for comments.",
kind: .lint,
nonTriggeringExamples: [
Example("""
// This is a comment
"""),
Example("""
/// Triple slash comment
"""),
Example("""
// Multiline double-slash
// comment
"""),
Example("""
/// Multiline triple-slash
/// comment
"""),
Example("""
/// Multiline triple-slash
/// - This is indented
"""),
Example("""
// - MARK: Mark comment
"""),
Example("""
/* Asterisk comment */
"""),
Example("""
/*
Multiline asterisk comment
*/
""")
],
triggeringExamples: [
Example("""
//↓Something
"""),
Example("""
//↓MARK
"""),
Example("""
//↓👨‍👨‍👦‍👦Something
"""),
Example("""
func a() {
//↓This needs refactoring
print("Something")
}
//↓We should improve above function
"""),
Example("""
///↓This is a comment
"""),
Example("""
/// Multiline triple-slash
///↓This line is incorrect, though
"""),
Example("""
//↓- MARK: Mark comment
""")
],
corrections: [
Example("//↓Something"): Example("// Something"),
Example("//↓- MARK: Mark comment"): Example("// - MARK: Mark comment"),
Example("""
/// Multiline triple-slash
///↓This line is incorrect, though
"""): Example("""
/// Multiline triple-slash
/// This line is incorrect, though
"""),
Example("""
func a() {
//↓This needs refactoring
print("Something")
}
//↓We should improve above function
"""): Example("""
func a() {
// This needs refactoring
print("Something")
}
// We should improve above function
""")
]
)

public func violationRanges(in file: SwiftLintFile) -> [NSRange] {
// Find all comment tokens in the file and regex search them for violations
let commentTokens = file.syntaxMap.tokens.filter { token in
guard let kind = token.kind else { return false }
return SyntaxKind.commentKinds.contains(kind)
}
return commentTokens
.compactMap { (token: SwiftLintSyntaxToken) -> [NSRange]? in
return file.stringView
.substringWithByteRange(token.range)
.map(StringView.init)
.map { commentBody in
// Look for 2-3 slash characters followed immediately by a non-whitespace, non-slash
// character (this is a violation)
regex(#"^(\/){2,3}[^\s\/]"#).matches(in: commentBody, options: .anchored).compactMap { result in
// Set the location to be directly before the first non-slash,
// non-whitespace character which was matched
return file.stringView.byteRangeToNSRange(
ByteRange(
// Safe to mix NSRange offsets with byte offsets here because the regex can't
// contain multi-byte characters
location: ByteCount(token.range.lowerBound.value + result.range.upperBound - 1),
length: 0
)
)
}
}
}
.flatMap { $0 }
}

public func validate(file: SwiftLintFile) -> [StyleViolation] {
return violationRanges(in: file).map { range in
StyleViolation(
ruleDescription: Self.description,
severity: configuration.severity,
location: Location(file: file, characterOffset: range.location)
)
}
}

public func substitution(for violationRange: NSRange, in file: SwiftLintFile) -> (NSRange, String)? {
return (violationRange, " ")
}
}
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
public struct OverridenSuperCallConfiguration: RuleConfiguration, Equatable {
private let defaultIncluded = [
//NSObject
// NSObject
"awakeFromNib()",
"prepareForInterfaceBuilder()",
//UICollectionViewLayout
// UICollectionViewLayout
"invalidateLayout()",
"invalidateLayout(with:)",
"invalidateLayoutWithContext(_:)",
//UIView
// UIView
"prepareForReuse()",
"updateConstraints()",
//UIViewController
// UIViewController
"addChildViewController(_:)",
"decodeRestorableState(with:)",
"decodeRestorableStateWithCoder(_:)",
Expand All @@ -27,7 +27,7 @@ public struct OverridenSuperCallConfiguration: RuleConfiguration, Equatable {
"viewDidLoad()",
"viewWillAppear(_:)",
"viewWillDisappear(_:)",
//XCTestCase
// XCTestCase
"setUp()",
"setUpWithError()",
"tearDown()",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ private extension SwiftLintFile {
return false
}

//First non-whitespace character should be "(" - otherwise it is not an anonymous closure
// First non-whitespace character should be "(" - otherwise it is not an anonymous closure
let afterBracketCode = closureCode.substring(from: closingBracketPosition + 1)
.trimmingCharacters(in: .whitespaces)
return afterBracketCode.first == "("
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public struct VerticalWhitespaceRule: CorrectableRule, ConfigurationProviderRule
let description = Self.description
let location = Location(file: file, characterOffset: currentLine.range.location)

//reports every line that is being deleted
// reports every line that is being deleted
corrections.append(Correction(ruleDescription: description, location: location))
continue // skips line
}
Expand Down
7 changes: 7 additions & 0 deletions Tests/LinuxMain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@ extension CommandTests {
]
}

extension CommentSpacingRuleTests {
static var allTests: [(String, (CommentSpacingRuleTests) -> () throws -> Void)] = [
("testWithDefaultConfiguration", testWithDefaultConfiguration)
]
}

extension CompilerProtocolInitRuleTests {
static var allTests: [(String, (CompilerProtocolInitRuleTests) -> () throws -> Void)] = [
("testWithDefaultConfiguration", testWithDefaultConfiguration),
Expand Down Expand Up @@ -1756,6 +1762,7 @@ XCTMain([
testCase(ColonRuleTests.allTests),
testCase(CommaRuleTests.allTests),
testCase(CommandTests.allTests),
testCase(CommentSpacingRuleTests.allTests),
testCase(CompilerProtocolInitRuleTests.allTests),
testCase(ComputedAccessorsOrderRuleTests.allTests),
testCase(ConditionalReturnsOnNewlineRuleTests.allTests),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ class CommaRuleTests: XCTestCase {
}
}

class CommentSpacingRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(CommentSpacingRule.description)
}
}

class ContainsOverFilterCountRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(ContainsOverFilterCountRule.description)
Expand Down
8 changes: 4 additions & 4 deletions Tests/SwiftLintFrameworkTests/CommandTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -257,22 +257,22 @@ class CommandTests: XCTestCase {
func testDisableAllOverridesSuperfluousDisableCommand() {
XCTAssert(
violations(
Example("//swiftlint:disable all\n// swiftlint:disable nesting\nprint(123)\n")
Example("// swiftlint:disable all\n// swiftlint:disable nesting\nprint(123)\n")
).isEmpty
)
XCTAssert(
violations(
Example("//swiftlint:disable all\n// swiftlint:disable:next nesting\nprint(123)\n")
Example("// swiftlint:disable all\n// swiftlint:disable:next nesting\nprint(123)\n")
).isEmpty
)
XCTAssert(
violations(
Example("//swiftlint:disable all\n// swiftlint:disable:this nesting\nprint(123)\n")
Example("// swiftlint:disable all\n// swiftlint:disable:this nesting\nprint(123)\n")
).isEmpty
)
XCTAssert(
violations(
Example("//swiftlint:disable all\n// swiftlint:disable:previous nesting\nprint(123)\n")
Example("// swiftlint:disable all\n// swiftlint:disable:previous nesting\nprint(123)\n")
).isEmpty
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class ExplicitTypeInterfaceRuleTests: XCTestCase {
verifyRule(description)
}

//swiftlint:disable function_body_length
// swiftlint:disable function_body_length
func testSwitchCaseDeclarations() {
guard SwiftVersion.current >= .fourDotOne else {
return
Expand Down
2 changes: 1 addition & 1 deletion Tests/SwiftLintFrameworkTests/LineEndingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class LineEndingTests: XCTestCase {
func testCarriageReturnDoesNotCauseError() {
XCTAssert(
violations(
Example("//swiftlint:disable all\r\nprint(123)\r\n")
Example("// swiftlint:disable all\r\nprint(123)\r\n")
).isEmpty
)
}
Expand Down
2 changes: 1 addition & 1 deletion Tests/SwiftLintFrameworkTests/ModifierOrderTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class ModifierOrderTests: XCTestCase {
"override"]])
}

//swiftlint:disable function_body_length
// swiftlint:disable function_body_length
func testAtPrefixedGroup() {
let descriptionOverride = RuleDescription(
identifier: "modifier_order",
Expand Down
3 changes: 2 additions & 1 deletion Tests/SwiftLintFrameworkTests/TestHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ extension XCTestCase {
XCTAssertEqual(
triggers.flatMap({ makeViolations($0.with(code: "/*\n " + $0.code + "\n */")) }).count,
commentDoesntViolate ? 0 : triggers.count,
"Violation(s) still triggered when code was nested inside comment",
file: (file), line: line
)
}
Expand Down Expand Up @@ -433,7 +434,7 @@ extension XCTestCase {
let violationsAtUnexpectedLocation = triggerViolations
.filter { !expectedLocations.contains($0.location) }
if !violationsAtUnexpectedLocation.isEmpty {
XCTFail("triggeringExample violate at unexpected location: \n" +
XCTFail("triggeringExample violated at unexpected location: \n" +
"\(render(violations: violationsAtUnexpectedLocation, in: trigger.code))",
file: trigger.file,
line: trigger.line)
Expand Down

0 comments on commit 2e26e9c

Please sign in to comment.