Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Added fileprivateRule #1489

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@
* Match `(Void)` as return type in the `void_return` rule.
[Anders Hasselqvist](https://github.com/nevil)

* Added `fileprivateRule` to check for top-level usages of `fileprivate` and recommend `private` instead. This is inline with SE-0169's goal "for `fileprivate` to be used rarely". There is a also an "strict" option that will mark every `fileprivate` as a violation.
[Jose Cheyo Jimenez](https://github.com/masters3d)
[#1469](https://github.com/realm/SwiftLint/issues/1469)
[#1058](https://github.com/realm/SwiftLint/issues/1058)

* Add `multiline_parameters` opt-in rule that warns to either keep
all the parameters of a method or function on the same line,
or one per line.
Expand Down
1 change: 1 addition & 0 deletions Source/SwiftLintFramework/Models/MasterRuleList.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public let masterRuleList = RuleList(rules: [
FatalErrorMessageRule.self,
FileHeaderRule.self,
FileLengthRule.self,
FileprivateRule.self,
FirstWhereRule.self,
ForWhereRule.self,
ForceCastRule.self,
Expand Down
53 changes: 53 additions & 0 deletions Source/SwiftLintFramework/Rules/FileprivateRule.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
//
// FileprivateRule.swift
// SwiftLint
//
// Created by Jose Cheyo Jimenez on 05/02/17.
// Copyright © 2017 Realm. All rights reserved.
//

import Foundation
import SourceKittenFramework

public struct FileprivateRule: Rule, ConfigurationProviderRule {
public var configuration = FileprivateConfiguration(strict: false)

public init() {}

public static let description = FileprivateConfiguration.fileprivateLimited

public func validate(file: File) -> [StyleViolation] {
if !configuration.strict {
let toplevel = file.structure.dictionary.substructure.flatMap({ $0.offset })
let syntaxTokens = file.syntaxMap.tokens
let violationOffsets = toplevel.flatMap { (offSet) -> Int? in
let parts = syntaxTokens.partitioned { offSet <= $0.offset }
guard let lastKind = parts.first.last,
lastKind.type == SyntaxKind.attributeBuiltin.rawValue,
// Cut the amount of name-look-ups by first checking the char count
lastKind.length == "fileprivate".bridge().length,
// Get the actual name of the attibute
let aclName = file.contents.bridge()
.substringWithByteRange(start:lastKind.offset, length: lastKind.length),
// fileprivate(set) is not possible at toplevel
aclName == "fileprivate"
else { return nil }
return offSet
}
return violationOffsets.map({ StyleViolation(
ruleDescription: FileprivateConfiguration.fileprivateLimited,
location: Location(file: file, byteOffset: $0))
})

} else { // Mark all fileprivate occurences as a violation
let fileprivates = file.match(pattern: "fileprivate", with: [.attributeBuiltin]).flatMap({
file.contents.bridge().NSRangeToByteRange(start: $0.location, length: $0.length)
}).map({ $0.location })
return fileprivates.map({ StyleViolation(
ruleDescription: FileprivateConfiguration.fileprivateDisallowed,
location: Location(file: file, byteOffset: $0))
})
}

}
}
2 changes: 1 addition & 1 deletion Source/SwiftLintFramework/Rules/IdentifierNameRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public struct IdentifierNameRule: ASTRule, ConfigurationProviderRule {
}
}

fileprivate extension String {
private extension String {
var isViolatingCase: Bool {
let secondIndex = characters.index(after: startIndex)
let firstCharacter = substring(to: secondIndex)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
//
// FileprivateConfiguration.swift
// SwiftLint
//
// Created by Jose Cheyo Jimenez on 05/02/17.
// Copyright © 2017 Realm. All rights reserved.
//

public struct FileprivateConfiguration: RuleConfiguration, Equatable {
private(set) var severityConfiguration = SeverityConfiguration(.warning)
private(set) var strict: Bool

public var consoleDescription: String {
return severityConfiguration.consoleDescription + ", strict: \(strict)"
}

public init(strict: Bool) {
self.strict = strict
}

public mutating func apply(configuration: Any) throws {
guard let configuration = configuration as? [String: Any] else {
throw ConfigurationError.unknownConfiguration
}

if let strict = configuration["strict"] as? Bool {
self.strict = strict
}

if let severityString = configuration["severity"] as? String {
try severityConfiguration.apply(configuration: severityString)
}
}

public static let fileprivateLimited = RuleDescription(
identifier: "fileprivate",
name: "Limit Fileprivate",
description: "Prefer private over fileprivate for top-level declarations",
nonTriggeringExamples: [
"extension String {}",
"private extension String {}",
"public \n enum MyEnum {}",
"open extension \n String {}",
"internal extension String {}",
"extension String {\nfileprivate func Something(){}\n}",
"class MyClass {\nfileprivate let myInt = 4\n}",
"class MyClass {\nfileprivate(set) var myInt = 4\n}",
"struct Outter {\nstruct Inter {\nfileprivate struct Inner {}\n}\n}"
],
triggeringExamples: [
"fileprivate enum MyEnum {}",
"fileprivate extension String {}",
"fileprivate \n extension String {}",
"fileprivate extension \n String {}",
"fileprivate class MyClass {\nfileprivate(set) var myInt = 4\n}",
"fileprivate extension String {}"
]
)

public static let fileprivateDisallowed = RuleDescription(
identifier: "fileprivate",
name: "Fileprivate Disallowed",
description: "Fileprivate should be rare. Consider refactoring",
nonTriggeringExamples: [
"extension String {}",
"private extension String {}",
"public \n extension String {}",
"open extension \n String {}",
"internal extension String {}",
""
],
triggeringExamples: [
"fileprivate extension String {}",
"fileprivate extension String {}",
"fileprivate \n extension String {}",
"fileprivate extension \n String {}",
"fileprivate extension String {}",
"extension String {\nfileprivate func Something(){}\n}",
"class MyClass {\nfileprivate let myInt = 4\n}",
"class MyClass {\nfileprivate(set) var myInt = 4\n}",
"struct Outter {\nstruct Inter {\nfileprivate struct Inner {}\n}\n}"
]
)

public static func == (lhs: FileprivateConfiguration,
rhs: FileprivateConfiguration) -> Bool {
return lhs.strict == rhs.strict &&
lhs.severityConfiguration == rhs.severityConfiguration
}
}
8 changes: 8 additions & 0 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
094385011D5D2894009168CF /* WeakDelegateRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 094384FF1D5D2382009168CF /* WeakDelegateRule.swift */; };
094385041D5D4F7C009168CF /* PrivateOutletRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 094385021D5D4F78009168CF /* PrivateOutletRule.swift */; };
1E18574B1EADBA51004F89F7 /* NoExtensionAccessModifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1E18574A1EADBA51004F89F7 /* NoExtensionAccessModifier.swift */; };
1E3C2D711EE36C6F00C8386D /* FileprivateRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1E3C2D701EE36C6F00C8386D /* FileprivateRule.swift */; };
1E3C2D731EE36D3500C8386D /* FileprivateConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1E3C2D721EE36D3500C8386D /* FileprivateConfiguration.swift */; };
1E82D5591D7775C7009553D7 /* ClosureSpacingRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1E82D5581D7775C7009553D7 /* ClosureSpacingRule.swift */; };
1EC163521D5992D900DD2928 /* VerticalWhitespaceRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1EC163511D5992D900DD2928 /* VerticalWhitespaceRule.swift */; };
1EF115921EB2AD5900E30140 /* ExplicitTopLevelACLRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1EF115911EB2AD5900E30140 /* ExplicitTopLevelACLRule.swift */; };
Expand Down Expand Up @@ -301,6 +303,8 @@
094384FF1D5D2382009168CF /* WeakDelegateRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = WeakDelegateRule.swift; sourceTree = "<group>"; };
094385021D5D4F78009168CF /* PrivateOutletRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PrivateOutletRule.swift; sourceTree = "<group>"; };
1E18574A1EADBA51004F89F7 /* NoExtensionAccessModifier.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NoExtensionAccessModifier.swift; sourceTree = "<group>"; };
1E3C2D701EE36C6F00C8386D /* FileprivateRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FileprivateRule.swift; sourceTree = "<group>"; };
1E3C2D721EE36D3500C8386D /* FileprivateConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FileprivateConfiguration.swift; sourceTree = "<group>"; };
1E82D5581D7775C7009553D7 /* ClosureSpacingRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ClosureSpacingRule.swift; sourceTree = "<group>"; };
1EC163511D5992D900DD2928 /* VerticalWhitespaceRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = VerticalWhitespaceRule.swift; sourceTree = "<group>"; };
1EF115911EB2AD5900E30140 /* ExplicitTopLevelACLRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ExplicitTopLevelACLRule.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -604,6 +608,7 @@
D43B04671E07228D004016AF /* ColonConfiguration.swift */,
67EB4DF81E4CC101004E9ACD /* CyclomaticComplexityConfiguration.swift */,
D4C4A3511DEFBBB700E0E04C /* FileHeaderConfiguration.swift */,
1E3C2D721EE36D3500C8386D /* FileprivateConfiguration.swift */,
47ACC8971E7DC74E0088EEB2 /* ImplicitlyUnwrappedOptionalConfiguration.swift */,
3B034B6C1E0BE544005D49A9 /* LineLengthConfiguration.swift */,
3BCC04D01C4F56D3006073C3 /* NameConfiguration.swift */,
Expand Down Expand Up @@ -894,6 +899,7 @@
C3DE5DAA1E7DF99B00761483 /* FatalErrorMessageRule.swift */,
D4C4A34D1DEA877200E0E04C /* FileHeaderRule.swift */,
E88DEA891B0992B300A66CB0 /* FileLengthRule.swift */,
1E3C2D701EE36C6F00C8386D /* FileprivateRule.swift */,
D42D2B371E09CC0D00CD7A2E /* FirstWhereRule.swift */,
E88DEA7F1B09903300A66CB0 /* ForceCastRule.swift */,
E816194D1BFBFEAB00946723 /* ForceTryRule.swift */,
Expand Down Expand Up @@ -1317,6 +1323,7 @@
D4B0226F1E0C75F9007E5297 /* VerticalParameterAlignmentRule.swift in Sources */,
E8BDE3FF1EDF91B6002EC12F /* RuleList.swift in Sources */,
D44254271DB9C15C00492EA4 /* SyntacticSugarRule.swift in Sources */,
1E3C2D731EE36D3500C8386D /* FileprivateConfiguration.swift in Sources */,
006204DC1E1E492F00FFFBE1 /* VerticalWhitespaceConfiguration.swift in Sources */,
E88198441BEA93D200333A11 /* ColonRule.swift in Sources */,
E809EDA11B8A71DF00399043 /* Configuration.swift in Sources */,
Expand Down Expand Up @@ -1385,6 +1392,7 @@
1EF115921EB2AD5900E30140 /* ExplicitTopLevelACLRule.swift in Sources */,
D41E7E0B1DF9DABB0065259A /* RedundantStringEnumValueRule.swift in Sources */,
E88DEA711B09847500A66CB0 /* ViolationSeverity.swift in Sources */,
1E3C2D711EE36C6F00C8386D /* FileprivateRule.swift in Sources */,
B2902A0C1D66815600BFCCF7 /* PrivateUnitTestRule.swift in Sources */,
D47A51101DB2DD4800A4CC21 /* AttributesRule.swift in Sources */,
CE8178ED1EAC039D0063186E /* UnusedOptionalBindingConfiguration.swift in Sources */,
Expand Down
2 changes: 2 additions & 0 deletions Tests/LinuxMain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,8 @@ extension RulesTests {
("testExtensionAccessModifier", testExtensionAccessModifier),
("testFatalErrorMessage", testFatalErrorMessage),
("testFileLength", testFileLength),
("testFileprivateRule", testFileprivateRule),
("testFileprivateRuleWithConfig", testFileprivateRuleWithConfig),
("testFirstWhere", testFirstWhere),
("testForceCast", testForceCast),
("testForceTry", testForceTry),
Expand Down
4 changes: 2 additions & 2 deletions Tests/SwiftLintFrameworkTests/ConfigurationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ class ConfigurationTests: XCTestCase {

// MARK: - ProjectMock Paths

fileprivate extension String {
private extension String {
func stringByAppendingPathComponent(_ pathComponent: String) -> String {
return bridge().appendingPathComponent(pathComponent)
}
Expand All @@ -276,7 +276,7 @@ extension XCTestCase {
}
}

fileprivate extension XCTestCase {
private extension XCTestCase {

var projectMockPathLevel0: String {
return bundlePath.stringByAppendingPathComponent("ProjectMock")
Expand Down
4 changes: 2 additions & 2 deletions Tests/SwiftLintFrameworkTests/LinterCacheTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import Foundation
@testable import SwiftLintFramework
import XCTest

fileprivate struct CacheTestHelper {
private struct CacheTestHelper {
fileprivate let configuration: Configuration

private let ruleList: RuleList
Expand Down Expand Up @@ -60,7 +60,7 @@ fileprivate struct CacheTestHelper {
}
}

fileprivate class TestFileManager: LintableFileManager {
private class TestFileManager: LintableFileManager {
fileprivate func filesToLint(inPath: String, rootDirectory: String? = nil) -> [String] {
return []
}
Expand Down
9 changes: 9 additions & 0 deletions Tests/SwiftLintFrameworkTests/RulesTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,15 @@ class RulesTests: XCTestCase {
testMultiByteOffsets: false)
}

func testFileprivateRule() {
verifyRule(FileprivateConfiguration.fileprivateLimited)
}

func testFileprivateRuleWithConfig() {
verifyRule(FileprivateConfiguration.fileprivateDisallowed,
ruleConfiguration: ["strict": true])
}

func testFirstWhere() {
verifyRule(FirstWhereRule.description)
}
Expand Down