From 13451778c9fc74a0358d59c9d658ade20d17191e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81s=20Cecilia=20Luque?= Date: Wed, 21 Aug 2019 02:00:53 +0200 Subject: [PATCH 1/6] Added the `allowed_paths_regex` subrule under the file existance rule. Now it is possible to specify the allowed paths in a project by using multiple regexes --- CHANGELOG.md | 2 +- Package.swift | 3 + .../Rules/FileExistenceOptions.swift | 15 ++-- .../ProjLintKit/Rules/FileExistenceRule.swift | 71 +++++++++++++++++++ .../Rules/FileExistenceRuleTests.swift | 68 ++++++++++++++++++ 5 files changed, 147 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9f67d0..cebab01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a ## [Unreleased] ### Added -- None. +- Added the `allowed_paths_regex` subrule under the file existance rule. Now it is possible to specify the allowed paths in a project by using multiple regexes. ### Changed - Replaced `lint_fail_level` configuration option with `strict` command line argument. Specify `--strict` or `-s` if you want the tool to fail on warnings. ### Deprecated diff --git a/Package.swift b/Package.swift index 21ccac1..00ff5d6 100644 --- a/Package.swift +++ b/Package.swift @@ -4,6 +4,9 @@ import PackageDescription let package = Package( name: "ProjLint", + platforms: [ + .macOS(.v10_11), + ], products: [ .executable(name: "projlint", targets: ["ProjLint"]), .library(name: "ProjLintKit", targets: ["ProjLintKit"]) diff --git a/Sources/ProjLintKit/Rules/FileExistenceOptions.swift b/Sources/ProjLintKit/Rules/FileExistenceOptions.swift index 5bbe073..e024854 100644 --- a/Sources/ProjLintKit/Rules/FileExistenceOptions.swift +++ b/Sources/ProjLintKit/Rules/FileExistenceOptions.swift @@ -3,19 +3,12 @@ import Foundation class FileExistenceOptions: RuleOptions { let existingPaths: [String]? let nonExistingPaths: [String]? + let allowedPathsRegex: [String]? override init(_ optionsDict: [String: Any], rule: Rule.Type) { - let existingPaths = RuleOptions.optionalStringArray(forOption: "existing_paths", in: optionsDict, rule: rule) - let nonExistingPaths = RuleOptions.optionalStringArray(forOption: "non_existing_paths", in: optionsDict, rule: rule) - - guard existingPaths != nil || nonExistingPaths != nil else { - print("Rule \(rule.identifier) must have at least one option specified.", level: .error) - exit(EX_USAGE) - } - - self.existingPaths = existingPaths - self.nonExistingPaths = nonExistingPaths - + self.existingPaths = RuleOptions.optionalStringArray(forOption: "existing_paths", in: optionsDict, rule: rule) + self.nonExistingPaths = RuleOptions.optionalStringArray(forOption: "non_existing_paths", in: optionsDict, rule: rule) + self.allowedPathsRegex = RuleOptions.optionalStringArray(forOption: "allowed_paths_regex", in: optionsDict, rule: rule) super.init(optionsDict, rule: rule) } } diff --git a/Sources/ProjLintKit/Rules/FileExistenceRule.swift b/Sources/ProjLintKit/Rules/FileExistenceRule.swift index 71322f8..07a2b88 100644 --- a/Sources/ProjLintKit/Rules/FileExistenceRule.swift +++ b/Sources/ProjLintKit/Rules/FileExistenceRule.swift @@ -1,4 +1,5 @@ import Foundation +import HandySwift struct FileExistenceRule: Rule { static let name: String = "File Existence" @@ -44,6 +45,76 @@ struct FileExistenceRule: Rule { } } + if let allowedPathsRegex = options.allowedPathsRegex { + let allowedPathsViolations = violationsForAllowedPaths(allowedPathsRegex, in: directory) + violations.append(contentsOf: allowedPathsViolations) + } + + return violations + } + + private func violationsForAllowedPaths(_ allowedPathsRegex: [String], in directory: URL) -> [Violation] { + var violations: [Violation] = [] + + // Start by getting an array of all files under the directory. + // After, remove all files that are allowed, until ending up with the list of notAllowedFiles + var notAllowedFiles = recursivelyGetFiles(at: directory) + + // Do not check for paths that are already linted by previous projlint rules + let existingPaths = options.existingPaths?.map { URL(fileURLWithPath: $0, relativeTo: directory) } ?? [] + let nonExistingPaths = options.nonExistingPaths?.map { URL(fileURLWithPath: $0, relativeTo: directory) } ?? [] + let pathsAlreadyLinted = existingPaths + nonExistingPaths + notAllowedFiles.removeAll { existingFile in + pathsAlreadyLinted.contains { $0.path == existingFile.path } + } + + for allowedPathPattern in allowedPathsRegex { + guard let allowedPathRegex = try? Regex("^\(allowedPathPattern)$") else { + let violation = Violation( + rule: self, + message: "The following regex is not valid: '\(allowedPathPattern)'", + level: .error + ) + violations.append(violation) + break + } + + notAllowedFiles.removeAll { allowedPathRegex.matches($0.relativePath) } + } + + notAllowedFiles.forEach { + let violation = FileViolation( + rule: self, + message: "File exists, but it mustn't.", + level: options.violationLevel(defaultTo: defaultViolationLevel), + path: $0.path + ) + violations.append(violation) + } + return violations } + + private func recursivelyGetFiles(at currentUrl: URL) -> [URL] { + var files: [URL] = [] + + let resourceKeys: [URLResourceKey] = [.creationDateKey, .isRegularFileKey] + let enumerator = FileManager.default.enumerator( + at: currentUrl, + includingPropertiesForKeys: resourceKeys + )! + + for case let fileUrl as URL in enumerator { + // Rationale: force-try is ok. This can never fail, as the resourceKeys passed here are also passed to the enumerator + // swiftlint:disable:next force_try + let resourceValues = try! fileUrl.resourceValues(forKeys: Set(resourceKeys)) + // Force-unwrap is ok: this can never fail, as the isRegularFileKey resource key is passed previously to the enumerator + if resourceValues.isRegularFile! { + let url = URL(fileURLWithPath: fileUrl.path.replacingOccurrences(of: "\(currentUrl.path)/", with: ""), relativeTo: currentUrl) + files.append(url) + } + } + + return files + } } diff --git a/Tests/ProjLintKitTests/Rules/FileExistenceRuleTests.swift b/Tests/ProjLintKitTests/Rules/FileExistenceRuleTests.swift index a482aaf..062fb35 100644 --- a/Tests/ProjLintKitTests/Rules/FileExistenceRuleTests.swift +++ b/Tests/ProjLintKitTests/Rules/FileExistenceRuleTests.swift @@ -39,4 +39,72 @@ final class FileExistenceRuleTests: XCTestCase { XCTAssert(violations.isEmpty) } } + + func testAllowedPathsWithValidRegex() { + resourcesLoaded([infoPlistResource]) { + let optionsDict = ["allowed_paths_regex": ["Sources/SuportingFiles/Info\\.plist"]] + let rule = FileExistenceRule(optionsDict) + + let violations = rule.violations(in: Resource.baseUrl) + XCTAssertEqual(violations.count, 0) + } + + resourcesLoaded([infoPlistResource]) { + let optionsDict = ["allowed_paths_regex": ["Sources/SuportingFiles/Info2\\.plist"]] + let rule = FileExistenceRule(optionsDict) + + let violations = rule.violations(in: Resource.baseUrl) + XCTAssertEqual(violations.count, 1) + XCTAssertEqual(violations.compactMap { ($0 as? FileViolation)?.path }, [infoPlistResource.path]) + XCTAssertEqual(violations.compactMap { ($0 as? FileViolation)?.message }, ["File exists, but it mustn\'t."]) + } + + resourcesLoaded([infoPlistResource]) { + let optionsDict = ["allowed_paths_regex": ["Sources/SuportingFiles/.*"]] + let rule = FileExistenceRule(optionsDict) + + let violations = rule.violations(in: Resource.baseUrl) + XCTAssertEqual(violations.count, 0) + } + + resourcesLoaded([infoPlistResource]) { + let optionsDict = ["allowed_paths_regex": [".*"]] + let rule = FileExistenceRule(optionsDict) + + let violations = rule.violations(in: Resource.baseUrl) + XCTAssertEqual(violations.count, 0) + } + + resourcesLoaded([infoPlistResource]) { + let optionsDict = ["allowed_paths_regex": [".*\\.png"]] + let rule = FileExistenceRule(optionsDict) + + let violations = rule.violations(in: Resource.baseUrl) + XCTAssertEqual(violations.count, 1) + XCTAssertEqual(violations.compactMap { ($0 as? FileViolation)?.path }, [infoPlistResource.path]) + XCTAssertEqual(violations.compactMap { ($0 as? FileViolation)?.message }, ["File exists, but it mustn\'t."]) + } + } + + func testAllowedPathsWithInvalidRegex() { + let optionsDict = ["allowed_paths_regex": ["["]] + let rule = FileExistenceRule(optionsDict) + + let violations = rule.violations(in: Resource.baseUrl) + XCTAssertEqual(violations.count, 1) + XCTAssertEqual(violations.map { $0.message }, ["The following regex is not valid: \'[\'"]) + } + + func testAllowedPathsWithSamePathInOtherRules() { + resourcesLoaded([infoPlistResource]) { + let optionsDict = [ + "existing_paths": [infoPlistResource.path], + "allowed_paths_regex": ["ThisIsARandomPathThatDoesNotMatch"] + ] + let rule = FileExistenceRule(optionsDict) + + let violations = rule.violations(in: Resource.baseUrl) + XCTAssertEqual(violations.count, 0) + } + } } From b1bd3944b67e1c5e4243d84508ac599e911f0f2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81s=20Cecilia=20Luque?= Date: Wed, 28 Aug 2019 00:55:27 +0200 Subject: [PATCH 2/6] Fix merge --- Sources/ProjLintKit/Rules/FileExistenceRule.swift | 2 +- Tests/ProjLintKitTests/Rules/FileExistenceRuleTests.swift | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/ProjLintKit/Rules/FileExistenceRule.swift b/Sources/ProjLintKit/Rules/FileExistenceRule.swift index ed8e0b3..645eae8 100644 --- a/Sources/ProjLintKit/Rules/FileExistenceRule.swift +++ b/Sources/ProjLintKit/Rules/FileExistenceRule.swift @@ -89,7 +89,7 @@ struct FileExistenceRule: Rule { rule: self, message: "File exists, but it mustn't.", level: options.violationLevel(defaultTo: defaultViolationLevel), - path: $0.path + url: $0 ) violations.append(violation) } diff --git a/Tests/ProjLintKitTests/Rules/FileExistenceRuleTests.swift b/Tests/ProjLintKitTests/Rules/FileExistenceRuleTests.swift index 978d0c2..d1be74b 100644 --- a/Tests/ProjLintKitTests/Rules/FileExistenceRuleTests.swift +++ b/Tests/ProjLintKitTests/Rules/FileExistenceRuleTests.swift @@ -55,7 +55,7 @@ final class FileExistenceRuleTests: XCTestCase { let violations = rule.violations(in: Resource.baseUrl) XCTAssertEqual(violations.count, 1) - XCTAssertEqual(violations.compactMap { ($0 as? FileViolation)?.path }, [infoPlistResource.path]) + XCTAssertEqual(violations.compactMap { ($0 as? FileViolation)?.url.relativePath }, [infoPlistResource.relativePath]) XCTAssertEqual(violations.compactMap { ($0 as? FileViolation)?.message }, ["File exists, but it mustn\'t."]) } @@ -81,7 +81,7 @@ final class FileExistenceRuleTests: XCTestCase { let violations = rule.violations(in: Resource.baseUrl) XCTAssertEqual(violations.count, 1) - XCTAssertEqual(violations.compactMap { ($0 as? FileViolation)?.path }, [infoPlistResource.path]) + XCTAssertEqual(violations.compactMap { ($0 as? FileViolation)?.url.relativePath }, [infoPlistResource.relativePath]) XCTAssertEqual(violations.compactMap { ($0 as? FileViolation)?.message }, ["File exists, but it mustn\'t."]) } } @@ -98,7 +98,7 @@ final class FileExistenceRuleTests: XCTestCase { func testAllowedPathsWithSamePathInOtherRules() { resourcesLoaded([infoPlistResource]) { let optionsDict = [ - "existing_paths": [infoPlistResource.path], + "existing_paths": [infoPlistResource.relativePath], "allowed_paths_regex": ["ThisIsARandomPathThatDoesNotMatch"] ] let rule = FileExistenceRule(optionsDict) From 6afc7fed7611b0e4a60b5175d31de9598025af79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81s=20Cecilia=20Luque?= Date: Wed, 28 Aug 2019 11:55:35 +0200 Subject: [PATCH 3/6] Fixes according to comments --- .../Rules/FileExistenceOptions.swift | 20 ++++++++++++++++--- .../ProjLintKit/Rules/FileExistenceRule.swift | 2 +- .../Rules/FileExistenceRuleTests.swift | 15 +++++++------- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/Sources/ProjLintKit/Rules/FileExistenceOptions.swift b/Sources/ProjLintKit/Rules/FileExistenceOptions.swift index e024854..433810f 100644 --- a/Sources/ProjLintKit/Rules/FileExistenceOptions.swift +++ b/Sources/ProjLintKit/Rules/FileExistenceOptions.swift @@ -6,9 +6,23 @@ class FileExistenceOptions: RuleOptions { let allowedPathsRegex: [String]? override init(_ optionsDict: [String: Any], rule: Rule.Type) { - self.existingPaths = RuleOptions.optionalStringArray(forOption: "existing_paths", in: optionsDict, rule: rule) - self.nonExistingPaths = RuleOptions.optionalStringArray(forOption: "non_existing_paths", in: optionsDict, rule: rule) - self.allowedPathsRegex = RuleOptions.optionalStringArray(forOption: "allowed_paths_regex", in: optionsDict, rule: rule) + let existingPaths = RuleOptions.optionalStringArray(forOption: "existing_paths", in: optionsDict, rule: rule) + let nonExistingPaths = RuleOptions.optionalStringArray(forOption: "non_existing_paths", in: optionsDict, rule: rule) + let allowedPathsRegex = RuleOptions.optionalStringArray(forOption: "allowed_paths_regex", in: optionsDict, rule: rule) + + guard + existingPaths != nil || + nonExistingPaths != nil || + allowedPathsRegex != nil + else { + print("Rule \(rule.identifier) must have at least one option specified.", level: .error) + exit(EX_USAGE) + } + + self.existingPaths = existingPaths + self.nonExistingPaths = nonExistingPaths + self.allowedPathsRegex = allowedPathsRegex + super.init(optionsDict, rule: rule) } } diff --git a/Sources/ProjLintKit/Rules/FileExistenceRule.swift b/Sources/ProjLintKit/Rules/FileExistenceRule.swift index 645eae8..6ad8371 100644 --- a/Sources/ProjLintKit/Rules/FileExistenceRule.swift +++ b/Sources/ProjLintKit/Rules/FileExistenceRule.swift @@ -104,7 +104,7 @@ struct FileExistenceRule: Rule { let enumerator = FileManager.default.enumerator( at: currentUrl, includingPropertiesForKeys: resourceKeys - )! + )! for case let fileUrl as URL in enumerator { // Rationale: force-try is ok. This can never fail, as the resourceKeys passed here are also passed to the enumerator diff --git a/Tests/ProjLintKitTests/Rules/FileExistenceRuleTests.swift b/Tests/ProjLintKitTests/Rules/FileExistenceRuleTests.swift index d1be74b..d0c7c71 100644 --- a/Tests/ProjLintKitTests/Rules/FileExistenceRuleTests.swift +++ b/Tests/ProjLintKitTests/Rules/FileExistenceRuleTests.swift @@ -42,7 +42,7 @@ final class FileExistenceRuleTests: XCTestCase { func testAllowedPathsWithValidRegex() { resourcesLoaded([infoPlistResource]) { - let optionsDict = ["allowed_paths_regex": ["Sources/SuportingFiles/Info\\.plist"]] + let optionsDict = ["allowed_paths_regex": [#"Sources/SuportingFiles/Info\.plist"#]] let rule = FileExistenceRule(optionsDict) let violations = rule.violations(in: Resource.baseUrl) @@ -50,7 +50,7 @@ final class FileExistenceRuleTests: XCTestCase { } resourcesLoaded([infoPlistResource]) { - let optionsDict = ["allowed_paths_regex": ["Sources/SuportingFiles/Info2\\.plist"]] + let optionsDict = ["allowed_paths_regex": [#"Sources/SuportingFiles/Info2\.plist"#]] let rule = FileExistenceRule(optionsDict) let violations = rule.violations(in: Resource.baseUrl) @@ -60,7 +60,7 @@ final class FileExistenceRuleTests: XCTestCase { } resourcesLoaded([infoPlistResource]) { - let optionsDict = ["allowed_paths_regex": ["Sources/SuportingFiles/.*"]] + let optionsDict = ["allowed_paths_regex": [#"Sources/SuportingFiles/.*"#]] let rule = FileExistenceRule(optionsDict) let violations = rule.violations(in: Resource.baseUrl) @@ -68,7 +68,7 @@ final class FileExistenceRuleTests: XCTestCase { } resourcesLoaded([infoPlistResource]) { - let optionsDict = ["allowed_paths_regex": [".*"]] + let optionsDict = ["allowed_paths_regex": [#".*"#]] let rule = FileExistenceRule(optionsDict) let violations = rule.violations(in: Resource.baseUrl) @@ -76,7 +76,7 @@ final class FileExistenceRuleTests: XCTestCase { } resourcesLoaded([infoPlistResource]) { - let optionsDict = ["allowed_paths_regex": [".*\\.png"]] + let optionsDict = ["allowed_paths_regex": [#".*\.png"#]] let rule = FileExistenceRule(optionsDict) let violations = rule.violations(in: Resource.baseUrl) @@ -87,7 +87,8 @@ final class FileExistenceRuleTests: XCTestCase { } func testAllowedPathsWithInvalidRegex() { - let optionsDict = ["allowed_paths_regex": ["["]] + let invalidRegex = #"["# + let optionsDict = ["allowed_paths_regex": [invalidRegex]] let rule = FileExistenceRule(optionsDict) let violations = rule.violations(in: Resource.baseUrl) @@ -99,7 +100,7 @@ final class FileExistenceRuleTests: XCTestCase { resourcesLoaded([infoPlistResource]) { let optionsDict = [ "existing_paths": [infoPlistResource.relativePath], - "allowed_paths_regex": ["ThisIsARandomPathThatDoesNotMatch"] + "allowed_paths_regex": [#"ThisIsARandomPathThatDoesNotMatch"#] ] let rule = FileExistenceRule(optionsDict) From 429e2a4daf7f505b453a31f0b232c9573376b897 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81s=20Cecilia=20Luque?= Date: Wed, 28 Aug 2019 12:06:01 +0200 Subject: [PATCH 4/6] Added resource test --- Tests/ProjLintKitTests/Globals/ResourceTests.swift | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 Tests/ProjLintKitTests/Globals/ResourceTests.swift diff --git a/Tests/ProjLintKitTests/Globals/ResourceTests.swift b/Tests/ProjLintKitTests/Globals/ResourceTests.swift new file mode 100644 index 0000000..544b0b6 --- /dev/null +++ b/Tests/ProjLintKitTests/Globals/ResourceTests.swift @@ -0,0 +1,12 @@ +import XCTest + +final class ResourceTest: XCTestCase { + private let resource = Resource( + path: "directory/Resource.swift", + contents: "" + ) + + func testRelativePath() { + XCTAssertEqual(resource.relativePath, "directory/Resource.swift") + } +} From d24b7a6a34ffac4dac31914c009281ea14e9591f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81s=20Cecilia=20Luque?= Date: Wed, 28 Aug 2019 12:10:33 +0200 Subject: [PATCH 5/6] Clean --- Sources/ProjLintKit/Rules/FileExistenceOptions.swift | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Sources/ProjLintKit/Rules/FileExistenceOptions.swift b/Sources/ProjLintKit/Rules/FileExistenceOptions.swift index 433810f..a04f612 100644 --- a/Sources/ProjLintKit/Rules/FileExistenceOptions.swift +++ b/Sources/ProjLintKit/Rules/FileExistenceOptions.swift @@ -10,11 +10,8 @@ class FileExistenceOptions: RuleOptions { let nonExistingPaths = RuleOptions.optionalStringArray(forOption: "non_existing_paths", in: optionsDict, rule: rule) let allowedPathsRegex = RuleOptions.optionalStringArray(forOption: "allowed_paths_regex", in: optionsDict, rule: rule) - guard - existingPaths != nil || - nonExistingPaths != nil || - allowedPathsRegex != nil - else { + let options = [existingPaths, nonExistingPaths, allowedPathsRegex] + guard options.contains(where: { $0 != nil }) else { print("Rule \(rule.identifier) must have at least one option specified.", level: .error) exit(EX_USAGE) } From daa444a32519e5fc6b5bb0679c955331be4423dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81s=20Cecilia=20Luque?= Date: Mon, 2 Sep 2019 14:35:07 +0200 Subject: [PATCH 6/6] Added rule to readme --- Rules.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Rules.md b/Rules.md index da845d0..b4199c9 100644 --- a/Rules.md +++ b/Rules.md @@ -104,6 +104,7 @@ Option | Type | Required? | Description --- | --- | --- | --- `existing_paths` | `[String]` | no | Files that must exist. `non_existing_paths` | `[String]` | no | Files that must not exist. +`allowed_paths_regex` | `[String]` | no | A list of regexes matching only allowed paths: files with a path that do not match the regex will trigger a violation.
Example @@ -120,6 +121,12 @@ rules: non_existing_paths: - Podfile - Podfile.lock + allowed_paths_regex: + - (Sources|Tests)/.+\.swift # Sources + - (Resources|Formula)/.+ # Other necessary resources + - \.(build|sourcery|git|templates)/.+ # Necessary files under hidden directories + - ProjLint\.xcodeproj/.+ # Xcode project + - '[^/]+' # Root files (needs quotation because the regex contains reserved yaml characters) ```