Add some linting improvements

This commit is contained in:
Pedro Piñera 2018-07-21 09:41:27 -04:00 committed by Pedro Piñera Buendía
parent adb157a939
commit 239ad8d1ec
6 changed files with 76 additions and 104 deletions

View File

@ -1,34 +1,23 @@
import Foundation
protocol GraphLinting: AnyObject {
/// Lints a projects graph.
///
/// - Parameter graph: projects graph to be linted.
/// - Returns: linting issues validating the graph.
func lint(graph: Graphing) -> [LintingIssue]
}
/// Lints the projects graph.
class GraphLinter: GraphLinting {
// MARK: - Attributes
/// Project validator.
let projectLinter: ProjectLinting
// MARK: - Init
/// Initializes the linter with its attributes.
///
/// - Parameter projectLinter: project linter.
init(projectLinter: ProjectLinting = ProjectLinter()) {
self.projectLinter = projectLinter
}
/// Lints a projects graph.
///
/// - Parameter graph: projects graph to be linted.
/// - Returns: linting issues validating the graph.
// MARK: - GraphLinting
func lint(graph: Graphing) -> [LintingIssue] {
var issues: [LintingIssue] = []
issues.append(contentsOf: graph.projects.flatMap(projectLinter.lint))

View File

@ -1,15 +0,0 @@
import Foundation
/// Protocol that represents an object used to lint other objects.
/// In the context of xpm, it's used to lint project models and verify
/// things that cannot be verified at compilation time.
protocol Linting {
/// Type of object that the linter lints.
associatedtype T
/// Lints an object thrown an error if the linting fails.
///
/// - Parameter object: object to be linted.
/// - Throws: an error if the linting fails.
func lint(_ object: T) throws
}

View File

@ -1,37 +1,28 @@
import Foundation
import xpmcore
/// Linting errors.
struct LintingError: FatalError, Equatable {
// MARK: - Attributes
/// Linting issues.
private let issues: [LintingIssue]
/// Initializes the error with the linting issues.
///
/// - Parameter issues: issues.
// MARK: - Init
init(issues: [LintingIssue]) {
self.issues = issues
}
/// Error description.
// MARK: - FatalError
var description: String {
return issues.map({ "- \($0.description)" }).joined(separator: "\n")
}
/// Error type.
var type: ErrorType {
return .abort
}
/// Compares two instances of LintingError.
///
/// - Parameters:
/// - lhs: first instance to be compared.
/// - rhs: second instance to be compared.
/// - Returns: true if the two instances are the same.
static func == (lhs: LintingError, rhs: LintingError) -> Bool {
return lhs.issues == rhs.issues
}
@ -39,10 +30,6 @@ struct LintingError: FatalError, Equatable {
/// Linting issue.
struct LintingIssue: CustomStringConvertible, Equatable {
/// Issue severities.
///
/// - warning: used for issues that the developer should be alerted about but that shouldn't error the process.
/// - error: used for issues that should error the process.
enum Severity: String {
case warning
case error
@ -50,17 +37,11 @@ struct LintingIssue: CustomStringConvertible, Equatable {
// MARK: - Attributes
/// Issue reason.
fileprivate let reason: String
/// Issue severity.
fileprivate let severity: Severity
/// Default LintingIssue constructor.
///
/// - Parameters:
/// - reason: issue reason.
/// - severity: issue severity
// MARK: - Init
init(reason: String, severity: Severity) {
self.reason = reason
self.severity = severity
@ -68,19 +49,12 @@ struct LintingIssue: CustomStringConvertible, Equatable {
// MARK: - CustomStringConvertible
/// Description.
var description: String {
return reason
}
// MARK: - Equatable
/// Compares two instances of LintingIssue.
///
/// - Parameters:
/// - lhs: first instance to be compared.
/// - rhs: second instance to be compared.
/// - Returns: true if the two instances are equal.
static func == (lhs: LintingIssue, rhs: LintingIssue) -> Bool {
return lhs.severity == rhs.severity &&
lhs.reason == rhs.reason
@ -90,11 +64,6 @@ struct LintingIssue: CustomStringConvertible, Equatable {
// MARK: - Array Extension (Linting issues)
extension Array where Element == LintingIssue {
/// Prints all the issues using the given printer and throws
/// if any of the issues is an error issue.
///
/// - Parameter printer: printer used to print the issues.
/// - Throws: an error if any of the issues is an error.
func printAndThrowIfNeeded(printer: Printing) throws {
if count == 0 { return }

View File

@ -2,40 +2,32 @@ import Basic
import Foundation
import xpmcore
/// Project linting protocol.
protocol ProjectLinting: AnyObject {
func lint(_ project: Project) -> [LintingIssue]
}
/// Lints the format of the project.
class ProjectLinter: ProjectLinting {
// MARK: - Attributes
/// Target linter.
let targetLinter: TargetLinting
/// Initialize the linter with its attributes.
///
/// - Parameter targetLinter: target linter.
// MARK: - Init
init(targetLinter: TargetLinting = TargetLinter()) {
self.targetLinter = targetLinter
}
/// Lints a project.
///
/// - Parameter project: project to be linted.
/// - Returns: all the linting issues found.
// MARK: - ProjectLinting
func lint(_ project: Project) -> [LintingIssue] {
var issues: [LintingIssue] = []
issues.append(contentsOf: lintTargets(project: project))
return issues
}
/// Lints the project targets.
///
/// - Parameter project: project whose targets will be linted.
/// - Returns: all linting issues.
// MARK: - Fileprivate
fileprivate func lintTargets(project: Project) -> [LintingIssue] {
var issues: [LintingIssue] = []
issues.append(contentsOf: project.targets.flatMap(targetLinter.lint))
@ -43,10 +35,6 @@ class ProjectLinter: ProjectLinting {
return issues
}
/// Verifies that there are no duplicated targets in the same project.
///
/// - Parameter project: project to be linted.
/// - Returns: issues if there are targets duplicated.
fileprivate func lintNotDuplicatedTargets(project: Project) -> [LintingIssue] {
var issues: [LintingIssue] = []
let duplicatedTargets = project.targets.map({ $0.name })

View File

@ -1,32 +1,25 @@
import Foundation
import xpmcore
/// Target linting protocol.
protocol TargetLinting: AnyObject {
/// It lints the given target.
///
/// - Parameter target: target to be valdiated.
/// - Throws: an error if the linting fails.
func lint(target: Target) -> [LintingIssue]
}
class TargetLinter: TargetLinting {
/// It lints the given target.
///
/// - Parameter target: target to be valdiated.
/// - Throws: an error if the linting fails.
// MARK: - TargetLinting
func lint(target: Target) -> [LintingIssue] {
var issues: [LintingIssue] = []
issues.append(contentsOf: lintHasSourceFiles(target: target))
issues.append(contentsOf: lintOneSourcesPhase(target: target))
issues.append(contentsOf: lintOneHeadersPhase(target: target))
issues.append(contentsOf: lintOneResourcesPhase(target: target))
issues.append(contentsOf: lintCopiedFiles(target: target))
return issues
}
/// Lints that the target contains source files.
///
/// - Parameter target: target to be validated.
/// - Throws: an error if the target doesn't contain any sources.
// MARK: - Fileprivate
fileprivate func lintHasSourceFiles(target: Target) -> [LintingIssue] {
let files = target.buildPhases.compactMap({ $0 as? SourcesBuildPhase })
@ -39,10 +32,6 @@ class TargetLinter: TargetLinting {
return issues
}
/// Verifies that the target has only one sources phase.
///
/// - Parameter target: target to be linted.
/// - Returns: a linting issue if the target has more than one source phase.
fileprivate func lintOneSourcesPhase(target: Target) -> [LintingIssue] {
let sourcesPhases = target.buildPhases
.filter({ $0 is SourcesBuildPhase })
@ -51,10 +40,6 @@ class TargetLinter: TargetLinting {
return [LintingIssue(reason: "The target \(target.name) has more than one sources build phase.", severity: .error)]
}
/// Verifies that the target has only one headers phase.
///
/// - Parameter target: target to be linted.
/// - Returns: a linting issue if the target has more than one headers phase.
fileprivate func lintOneHeadersPhase(target: Target) -> [LintingIssue] {
let headerPhases = target.buildPhases
.filter({ $0 is HeadersBuildPhase })
@ -62,4 +47,29 @@ class TargetLinter: TargetLinting {
if headerPhases <= 1 { return [] }
return [LintingIssue(reason: "The target \(target.name) has more than one headers build phase.", severity: .error)]
}
fileprivate func lintOneResourcesPhase(target: Target) -> [LintingIssue] {
let resourcesPhase = target.buildPhases
.filter({ $0 is ResourcesBuildPhase })
.count
if resourcesPhase <= 1 { return [] }
return [LintingIssue(reason: "The target \(target.name) has more than one resources build phase.", severity: .error)]
}
fileprivate func lintCopiedFiles(target: Target) -> [LintingIssue] {
guard let resourcesPhase = target.buildPhases.compactMap({ $0 as? ResourcesBuildPhase }).first else {
return []
}
var issues: [LintingIssue] = []
let buildFiles = resourcesPhase.buildFiles.compactMap({ $0 as? ResourcesBuildFile }).flatMap({ $0.paths })
let infoPlists = buildFiles.filter({ $0.asString.contains("Info.plist") })
let entitlements = buildFiles.filter({ $0.asString.contains(".entitlements") })
issues.append(contentsOf: infoPlists.map({ LintingIssue(reason: "Info.plist at path \($0.asString) being copied into the target \(target.name) product.", severity: .warning) }))
issues.append(contentsOf: entitlements.map({ LintingIssue(reason: "Entitlements file at path \($0.asString) being copied into the target \(target.name) product.", severity: .warning) }))
return issues
}
}

View File

@ -1,3 +1,4 @@
import Basic
import Foundation
import XCTest
@testable import xpmkit
@ -28,6 +29,16 @@ final class TargetLinterTests: XCTestCase {
XCTAssertTrue(got.contains(LintingIssue(reason: "The target \(target.name) has more than one sources build phase.", severity: .error)))
}
func test_lint_when_more_than_one_resources_build_phase() {
let buildPhase = ResourcesBuildPhase(buildFiles: [])
let buildPhases: [BuildPhase] = [buildPhase, buildPhase]
let target = Target.test(buildPhases: buildPhases)
let got = subject.lint(target: target)
XCTAssertTrue(got.contains(LintingIssue(reason: "The target \(target.name) has more than one resources build phase.", severity: .error)))
}
func test_lint_when_more_than_one_headers_build_phase() {
let buildPhase = HeadersBuildPhase(buildFiles: [])
let buildPhases: [BuildPhase] = [buildPhase, buildPhase]
@ -37,4 +48,24 @@ final class TargetLinterTests: XCTestCase {
XCTAssertTrue(got.contains(LintingIssue(reason: "The target \(target.name) has more than one headers build phase.", severity: .error)))
}
func test_lint_when_a_infoplist_file_is_being_copied() {
let path = AbsolutePath("/Info.plist")
let buildPhase = ResourcesBuildPhase(buildFiles: [ResourcesBuildFile([path])])
let target = Target.test(buildPhases: [buildPhase])
let got = subject.lint(target: target)
XCTAssertTrue(got.contains(LintingIssue(reason: "Info.plist at path \(path.asString) being copied into the target \(target.name) product.", severity: .warning)))
}
func test_lint_when_a_entitlements_file_is_being_copied() {
let path = AbsolutePath("/App.entitlements")
let buildPhase = ResourcesBuildPhase(buildFiles: [ResourcesBuildFile([path])])
let target = Target.test(buildPhases: [buildPhase])
let got = subject.lint(target: target)
XCTAssertTrue(got.contains(LintingIssue(reason: "Entitlements file at path \(path.asString) being copied into the target \(target.name) product.", severity: .warning)))
}
}