Skip to content

Commit

Permalink
Add linting for mismatching build configurations in a workspace (#474)
Browse files Browse the repository at this point in the history
Resolves #467

- All projects' build configurations are now validated against the top level projects
- In the event any are missing or are mismatching (e.g. have a mismatching variant) the linter will now flag them

Example:

```
$ tuist generate
The following issues have been found:
  - The project 'Framework2' has missing or mismatching configurations. It has [Debug (debug), Release (release)], other projects have  [Beta (release), Debug (debug), Release (release)].
```

Test Plan:

- Run `tuist generate` within `fixtures/ios_app_with_multi_configs`
- Comment out any of the build configurations defined in `App`, `Framework1` or `Framework2`
- Re-run `tuist generate`
- Verify a lint warning is displayed
  • Loading branch information
kwridan committed Aug 4, 2019
1 parent 6739265 commit a3684c6
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -14,6 +14,7 @@ Please, check out guidelines: https://keepachangelog.com/en/1.0.0/
- Adding ability to re-generate individual projects https://github.com/tuist/tuist/pull/457 by @kwridan
- Support multiple header paths https://github.com/tuist/tuist/pull/459 by @adamkhazi
- Allow specifying multiple configurations within project manifests https://github.com/tuist/tuist/pull/451 by @kwridan
- Add linting for mismatching build configurations in a workspace https://github.com/tuist/tuist/pull/474 by @kwridan

### Fixed

Expand Down
26 changes: 26 additions & 0 deletions Sources/TuistGenerator/Linter/GraphLinter.swift
Expand Up @@ -38,6 +38,7 @@ class GraphLinter: GraphLinting {
var issues: [LintingIssue] = []
issues.append(contentsOf: graph.projects.flatMap(projectLinter.lint))
issues.append(contentsOf: lintDependencies(graph: graph))
issues.append(contentsOf: lintMismatchingConfigurations(graph: graph))
return issues
}

Expand Down Expand Up @@ -148,6 +149,30 @@ class GraphLinter: GraphLinting {
return [issue]
}

private func lintMismatchingConfigurations(graph: Graphing) -> [LintingIssue] {
let entryNodeProjects = graph.entryNodes.compactMap { $0 as? TargetNode }.map { $0.project }

let knownConfigurations = entryNodeProjects.reduce(into: Set()) {
$0.formUnion(Set($1.settings.configurations.keys))
}

let projectBuildConfigurations = graph.projects.map {
(name: $0.name, buildConfigurations: Set($0.settings.configurations.keys))
}

let mismatchingBuildConfigurations = projectBuildConfigurations.filter {
!knownConfigurations.isSubset(of: $0.buildConfigurations)
}

return mismatchingBuildConfigurations.map {
let expectedConfigurations = knownConfigurations.sorted()
let configurations = $0.buildConfigurations.sorted()
let reason = "The project '\($0.name)' has missing or mismatching configurations. It has \(configurations), other projects have \(expectedConfigurations)"
return LintingIssue(reason: reason,
severity: .warning)
}
}

struct LintableTarget: Equatable, Hashable {
let platform: Platform
let product: Product
Expand All @@ -162,6 +187,7 @@ class GraphLinter: GraphLinting {
LintableTarget(platform: .iOS, product: .staticFramework),
LintableTarget(platform: .iOS, product: .bundle),
// LintableTarget(platform: .iOS, product: .appExtension),
// LintableTarget(platform: .iOS, product: .appExtension),
// LintableTarget(platform: .iOS, product: .messagesExtension),
// LintableTarget(platform: .iOS, product: .stickerPack),
// LintableTarget(platform: .watchOS, product: .watch2App),
Expand Down
6 changes: 6 additions & 0 deletions Sources/TuistGenerator/Models/BuildConfiguration.swift
Expand Up @@ -54,3 +54,9 @@ extension BuildConfiguration: Comparable {
extension BuildConfiguration: XcodeRepresentable {
public var xcodeValue: String { return name }
}

extension BuildConfiguration: CustomStringConvertible {
public var description: String {
return "\(name) (\(variant.rawValue))"
}
}
Expand Up @@ -45,14 +45,19 @@ extension Graph {
/// The `dependencies` property is used to define the dependencies explicitly.
/// All targets need to be listed even if they don't have any dependencies.
static func create(projects: [Project] = [],
entryNodes: [Target]? = nil,
dependencies: [(project: Project, target: Target, dependencies: [Target])]) -> Graph {
let targetNodes = createTargetNodes(dependencies: dependencies)

let entryNodes = entryNodes.map { entryNodes in
targetNodes.filter { entryNodes.contains($0.target) }
}

let cache = GraphLoaderCache()
let graph = Graph.test(name: projects.first?.name ?? "Test",
entryPath: projects.first?.path ?? "/test/path",
cache: cache,
entryNodes: targetNodes)
entryNodes: entryNodes ?? targetNodes)

targetNodes.forEach { cache.add(targetNode: $0) }
projects.forEach { cache.add(project: $0) }
Expand Down
115 changes: 115 additions & 0 deletions Tests/TuistGeneratorTests/Linter/GraphLinterTests.swift
Expand Up @@ -247,4 +247,119 @@ final class GraphLinterTests: XCTestCase {
// Then
XCTAssertFalse(result.isEmpty)
}

func test_lint_missingProjectConfigurationsFromDependencyProjects() throws {
// Given
let customConfigurations: [BuildConfiguration: Configuration?] = [
.debug("Debug"): nil,
.debug("Testing"): nil,
.release("Beta"): nil,
.release("Release"): nil,
]
let targetA = Target.empty(name: "TargetA", product: .framework)
let projectA = Project.empty(path: "/path/to/a", name: "ProjectA", settings: Settings(configurations: customConfigurations))

let targetB = Target.empty(name: "TargetB", product: .framework)
let projectB = Project.empty(path: "/path/to/b", name: "ProjectB", settings: Settings(configurations: customConfigurations))

let targetC = Target.empty(name: "TargetC", product: .framework)
let projectC = Project.empty(path: "/path/to/c", name: "ProjectC", settings: .default)

let graph = Graph.create(projects: [projectA, projectB, projectC],
dependencies: [
(project: projectA, target: targetA, dependencies: [targetB]),
(project: projectB, target: targetB, dependencies: [targetC]),
(project: projectC, target: targetC, dependencies: []),
])

// When
let result = subject.lint(graph: graph)

// Then
XCTAssertEqual(result, [
LintingIssue(reason: "The project 'ProjectC' has missing or mismatching configurations. It has [Debug (debug), Release (release)], other projects have [Beta (release), Debug (debug), Release (release), Testing (debug)]",
severity: .warning),
])
}

func test_lint_mismatchingProjectConfigurationsFromDependencyProjects() throws {
// Given
let customConfigurations: [BuildConfiguration: Configuration?] = [
.debug("Debug"): nil,
.debug("Testing"): nil,
.release("Beta"): nil,
.release("Release"): nil,
]
let targetA = Target.empty(name: "TargetA", product: .framework)
let projectA = Project.empty(path: "/path/to/a", name: "ProjectA", settings: Settings(configurations: customConfigurations))

let targetB = Target.empty(name: "TargetB", product: .framework)
let projectB = Project.empty(path: "/path/to/b", name: "ProjectB", settings: Settings(configurations: customConfigurations))

let mismatchingConfigurations: [BuildConfiguration: Configuration?] = [
.release("Debug"): nil,
.release("Testing"): nil,
.release("Beta"): nil,
.release("Release"): nil,
]
let targetC = Target.empty(name: "TargetC", product: .framework)
let projectC = Project.empty(path: "/path/to/c", name: "ProjectC", settings: Settings(configurations: mismatchingConfigurations))

let graph = Graph.create(projects: [projectA, projectB, projectC],
entryNodes: [targetA],
dependencies: [
(project: projectA, target: targetA, dependencies: [targetB]),
(project: projectB, target: targetB, dependencies: [targetC]),
(project: projectC, target: targetC, dependencies: []),
])

// When
let result = subject.lint(graph: graph)

// Then
XCTAssertEqual(result, [
LintingIssue(reason: "The project 'ProjectC' has missing or mismatching configurations. It has [Beta (release), Debug (release), Release (release), Testing (release)], other projects have [Beta (release), Debug (debug), Release (release), Testing (debug)]",
severity: .warning),
])
}

func test_lint_doesNotFlagDependenciesWithExtraConfigurations() throws {
// Lower level dependencies could be shared by projects in different workspaces as such
// it is ok for them to contain more configurations than the entry node projects

// Given
let customConfigurations: [BuildConfiguration: Configuration?] = [
.debug("Debug"): nil,
.release("Beta"): nil,
.release("Release"): nil,
]
let targetA = Target.empty(name: "TargetA", product: .framework)
let projectA = Project.empty(path: "/path/to/a", name: "ProjectA", settings: Settings(configurations: customConfigurations))

let targetB = Target.empty(name: "TargetB", product: .framework)
let projectB = Project.empty(path: "/path/to/b", name: "ProjectB", settings: Settings(configurations: customConfigurations))

let additionalConfigurations: [BuildConfiguration: Configuration?] = [
.debug("Debug"): nil,
.debug("Testing"): nil,
.release("Beta"): nil,
.release("Release"): nil,
]
let targetC = Target.empty(name: "TargetC", product: .framework)
let projectC = Project.empty(path: "/path/to/c", name: "ProjectC", settings: Settings(configurations: additionalConfigurations))

let graph = Graph.create(projects: [projectA, projectB, projectC],
entryNodes: [targetA],
dependencies: [
(project: projectA, target: targetA, dependencies: [targetB]),
(project: projectB, target: targetB, dependencies: [targetC]),
(project: projectC, target: targetC, dependencies: []),
])

// When
let result = subject.lint(graph: graph)

// Then
XCTAssertEqual(result, [])
}
}

0 comments on commit a3684c6

Please sign in to comment.