diff --git a/CHANGELOG.md b/CHANGELOG.md index 079405fde..8062ee1b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,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. diff --git a/Source/SwiftLintFramework/Models/MasterRuleList.swift b/Source/SwiftLintFramework/Models/MasterRuleList.swift index 11dd1734e..de3412d26 100644 --- a/Source/SwiftLintFramework/Models/MasterRuleList.swift +++ b/Source/SwiftLintFramework/Models/MasterRuleList.swift @@ -35,6 +35,7 @@ public let masterRuleList = RuleList(rules: [ FatalErrorMessageRule.self, FileHeaderRule.self, FileLengthRule.self, + FileprivateRule.self, FirstWhereRule.self, ForWhereRule.self, ForceCastRule.self, diff --git a/Source/SwiftLintFramework/Rules/FileprivateRule.swift b/Source/SwiftLintFramework/Rules/FileprivateRule.swift new file mode 100644 index 000000000..76866e85a --- /dev/null +++ b/Source/SwiftLintFramework/Rules/FileprivateRule.swift @@ -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)) + }) + } + + } +} diff --git a/Source/SwiftLintFramework/Rules/IdentifierNameRule.swift b/Source/SwiftLintFramework/Rules/IdentifierNameRule.swift index 1a973c6bc..22a26ffb3 100644 --- a/Source/SwiftLintFramework/Rules/IdentifierNameRule.swift +++ b/Source/SwiftLintFramework/Rules/IdentifierNameRule.swift @@ -116,7 +116,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) diff --git a/Source/SwiftLintFramework/Rules/RuleConfigurations/FileprivateConfiguration.swift b/Source/SwiftLintFramework/Rules/RuleConfigurations/FileprivateConfiguration.swift new file mode 100644 index 000000000..06e295009 --- /dev/null +++ b/Source/SwiftLintFramework/Rules/RuleConfigurations/FileprivateConfiguration.swift @@ -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 + } +} diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index db5486712..2b3a62f5b 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -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 */; }; @@ -304,6 +306,8 @@ 094384FF1D5D2382009168CF /* WeakDelegateRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = WeakDelegateRule.swift; sourceTree = ""; }; 094385021D5D4F78009168CF /* PrivateOutletRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PrivateOutletRule.swift; sourceTree = ""; }; 1E18574A1EADBA51004F89F7 /* NoExtensionAccessModifier.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NoExtensionAccessModifier.swift; sourceTree = ""; }; + 1E3C2D701EE36C6F00C8386D /* FileprivateRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FileprivateRule.swift; sourceTree = ""; }; + 1E3C2D721EE36D3500C8386D /* FileprivateConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FileprivateConfiguration.swift; sourceTree = ""; }; 1E82D5581D7775C7009553D7 /* ClosureSpacingRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ClosureSpacingRule.swift; sourceTree = ""; }; 1EC163511D5992D900DD2928 /* VerticalWhitespaceRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = VerticalWhitespaceRule.swift; sourceTree = ""; }; 1EF115911EB2AD5900E30140 /* ExplicitTopLevelACLRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ExplicitTopLevelACLRule.swift; sourceTree = ""; }; @@ -610,6 +614,7 @@ D43B04671E07228D004016AF /* ColonConfiguration.swift */, 67EB4DF81E4CC101004E9ACD /* CyclomaticComplexityConfiguration.swift */, D4C4A3511DEFBBB700E0E04C /* FileHeaderConfiguration.swift */, + 1E3C2D721EE36D3500C8386D /* FileprivateConfiguration.swift */, 47ACC8971E7DC74E0088EEB2 /* ImplicitlyUnwrappedOptionalConfiguration.swift */, 3B034B6C1E0BE544005D49A9 /* LineLengthConfiguration.swift */, 3BCC04D01C4F56D3006073C3 /* NameConfiguration.swift */, @@ -902,6 +907,7 @@ C3DE5DAA1E7DF99B00761483 /* FatalErrorMessageRule.swift */, D4C4A34D1DEA877200E0E04C /* FileHeaderRule.swift */, E88DEA891B0992B300A66CB0 /* FileLengthRule.swift */, + 1E3C2D701EE36C6F00C8386D /* FileprivateRule.swift */, D42D2B371E09CC0D00CD7A2E /* FirstWhereRule.swift */, E88DEA7F1B09903300A66CB0 /* ForceCastRule.swift */, E816194D1BFBFEAB00946723 /* ForceTryRule.swift */, @@ -1326,6 +1332,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 */, @@ -1394,6 +1401,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 */, diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index 2758b3860..dba125ba4 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -323,6 +323,8 @@ extension RulesTests { ("testExtensionAccessModifier", testExtensionAccessModifier), ("testFatalErrorMessage", testFatalErrorMessage), ("testFileLength", testFileLength), + ("testFileprivateRule", testFileprivateRule), + ("testFileprivateRuleWithConfig", testFileprivateRuleWithConfig), ("testFirstWhere", testFirstWhere), ("testForceCast", testForceCast), ("testForceTry", testForceTry), diff --git a/Tests/SwiftLintFrameworkTests/ConfigurationTests.swift b/Tests/SwiftLintFrameworkTests/ConfigurationTests.swift index 830916959..78f41ec40 100644 --- a/Tests/SwiftLintFrameworkTests/ConfigurationTests.swift +++ b/Tests/SwiftLintFrameworkTests/ConfigurationTests.swift @@ -269,7 +269,7 @@ class ConfigurationTests: XCTestCase { // MARK: - ProjectMock Paths -fileprivate extension String { +private extension String { func stringByAppendingPathComponent(_ pathComponent: String) -> String { return bridge().appendingPathComponent(pathComponent) } @@ -285,7 +285,7 @@ extension XCTestCase { } } -fileprivate extension XCTestCase { +private extension XCTestCase { var projectMockPathLevel0: String { return bundlePath.stringByAppendingPathComponent("ProjectMock") diff --git a/Tests/SwiftLintFrameworkTests/LinterCacheTests.swift b/Tests/SwiftLintFrameworkTests/LinterCacheTests.swift index 13a87d664..e05b7b7db 100644 --- a/Tests/SwiftLintFrameworkTests/LinterCacheTests.swift +++ b/Tests/SwiftLintFrameworkTests/LinterCacheTests.swift @@ -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 @@ -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 [] } diff --git a/Tests/SwiftLintFrameworkTests/RulesTests.swift b/Tests/SwiftLintFrameworkTests/RulesTests.swift index 5d2d00012..0babf255e 100644 --- a/Tests/SwiftLintFrameworkTests/RulesTests.swift +++ b/Tests/SwiftLintFrameworkTests/RulesTests.swift @@ -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) }