dnrops
/
tuist
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Update static dependency lint rule (#360) 

Resolves https://github.com/tuist/tuist/issues/337

- The graph linter didn't account for which targets could link static products
- This led to raising an incorrect lint warning
- A `canLinkStaticProducts` method has been added to share rules between the linter and graph during link generation
- `GraphLinter` has been updated to no longer keep state internally

Test Plan:

- Run unit tests via `swift lint`
- Run acceptance tests via `bundle exec rake features`
- Manually generate `fixtures/ios_app_with_static_frameworks` via `tuist generate`
- Verify no lint warnings are displayed
This commit is contained in:
Kas 2019-05-21 22:14:21 +01:00 committed by GitHub
parent c87a1cda62
commit 80dc496c8a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 213 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -23,6 +23,7 @@ Please, check out guidelines: https://keepachangelog.com/en/1.0.0/
- Supporting bundle target dependencies that reside in different projects (in `TuistGenerator`) https://github.com/tuist/tuist/pull/348 by @kwridan
- Fixing header paths including folders and non-header files https://github.com/tuist/tuist/pull/356 by @kwridan
- Fix duplicate localized resource files https://github.com/tuist/tuist/pull/363 by @kwridan
- Update static dependency lint rule https://github.com/tuist/tuist/pull/360 by @kwridan
## 0.14.0

9
Sources/TuistGenerator/Graph/Graph.swift
View File

@ -143,7 +143,7 @@ class Graph: Graphing {
var references: [DependencyReference] = []
/// Precompiled libraries and frameworks
// Precompiled libraries and frameworks
let precompiledLibrariesAndFrameworks = targetNode.precompiledDependencies
.map(\.path)
@ -151,12 +151,9 @@ class Graph: Graphing {
references.append(contentsOf: precompiledLibrariesAndFrameworks)
switch targetNode.target.product {
case .staticLibrary, .dynamicLibrary, .staticFramework, .bundle:
// Ignore the products, they do not want to directly link the static libraries, the top level bundles will be responsible.
break
case .app, .unitTests, .uiTests, .framework:
// Static libraries and frameworks
if targetNode.target.canLinkStaticProducts() {
let staticLibraries = findAll(targetNode: targetNode, test: isStaticLibrary, skip: isFramework)
.lazy
.map(\.target.productNameWithExtension)

65
Sources/TuistGenerator/Linter/GraphLinter.swift
View File

@ -20,20 +20,18 @@ class GraphLinter: GraphLinting {
}
struct StaticDepedencyWarning: Hashable {
let targetNode: TargetNode
let linkingStaticTargetNode: TargetNode
let fromTargetNode: TargetNode
let toTargetNode: TargetNode
func hash(into hasher: inout Hasher) {
hasher.combine(linkingStaticTargetNode)
hasher.combine(toTargetNode)
}
static func == (lhs: StaticDepedencyWarning, rhs: StaticDepedencyWarning) -> Bool {
return lhs.linkingStaticTargetNode == rhs.linkingStaticTargetNode
return lhs.toTargetNode == rhs.toTargetNode
}
}
var linkedStaticProducts = Set<StaticDepedencyWarning>()
// MARK: - GraphLinting
func lint(graph: Graphing) -> [LintingIssue] {
@ -48,8 +46,11 @@ class GraphLinter: GraphLinting {
private func lintDependencies(graph: Graphing) -> [LintingIssue] {
var issues: [LintingIssue] = []
var evaluatedNodes: [GraphNode] = []
var linkedStaticProducts = Set<StaticDepedencyWarning>()
graph.entryNodes.forEach {
issues.append(contentsOf: lintGraphNode(node: $0, evaluatedNodes: &evaluatedNodes))
issues.append(contentsOf: lintGraphNode(node: $0,
evaluatedNodes: &evaluatedNodes,
linkedStaticProducts: &linkedStaticProducts))
}
issues.append(contentsOf: lintCarthageDependencies(graph: graph))
@ -76,7 +77,9 @@ class GraphLinter: GraphLinting {
return issues
}
private func lintGraphNode(node: GraphNode, evaluatedNodes: inout [GraphNode]) -> [LintingIssue] {
private func lintGraphNode(node: GraphNode,
evaluatedNodes: inout [GraphNode],
linkedStaticProducts: inout Set<StaticDepedencyWarning>) -> [LintingIssue] {
var issues: [LintingIssue] = []
defer { evaluatedNodes.append(node) }
@ -85,27 +88,21 @@ class GraphLinter: GraphLinting {
targetNode.dependencies.forEach { toNode in
if let toTargetNode = toNode as? TargetNode {
if toTargetNode.target.product.isStatic {
let warning = StaticDepedencyWarning(targetNode: targetNode,
linkingStaticTargetNode: toTargetNode)
let (inserted, oldMember) = linkedStaticProducts.insert(warning)
if inserted == false {
let reason = "Target \(toTargetNode.target.name) has been linked against \(oldMember.targetNode.target.name) and \(targetNode.target.name), it is a static product so may introduce unwanted side effects."
let issue = LintingIssue(reason: reason, severity: .warning)
issues.append(issue)
}
}
issues.append(contentsOf: lintDependency(from: targetNode, to: toTargetNode))
issues.append(contentsOf: lintDependency(from: targetNode,
to: toTargetNode,
linkedStaticProducts: &linkedStaticProducts))
}
issues.append(contentsOf: lintGraphNode(node: toNode, evaluatedNodes: &evaluatedNodes))
issues.append(contentsOf: lintGraphNode(node: toNode,
evaluatedNodes: &evaluatedNodes,
linkedStaticProducts: &linkedStaticProducts))
}
return issues
}
private func lintDependency(from: TargetNode, to: TargetNode) -> [LintingIssue] {
private func lintDependency(from: TargetNode,
to: TargetNode,
linkedStaticProducts: inout Set<StaticDepedencyWarning>) -> [LintingIssue] {
var issues: [LintingIssue] = []
let fromTarget = LintableTarget(platform: from.target.platform,
@ -126,9 +123,31 @@ class GraphLinter: GraphLinting {
issues.append(issue)
}
issues.append(contentsOf: lintStaticDependencies(from: from,
to: to,
linkedStaticProducts: &linkedStaticProducts))
return issues
}
private func lintStaticDependencies(from: TargetNode,
to: TargetNode,
linkedStaticProducts: inout Set<StaticDepedencyWarning>) -> [LintingIssue] {
guard to.target.product.isStatic, from.target.canLinkStaticProducts() else {
return []
}
let warning = StaticDepedencyWarning(fromTargetNode: from,
toTargetNode: to)
let (inserted, oldMember) = linkedStaticProducts.insert(warning)
guard inserted == false else {
return []
}
let reason = "Target \(to.target.name) has been linked against \(oldMember.fromTargetNode.target.name) and \(from.target.name), it is a static product so may introduce unwanted side effects."
let issue = LintingIssue(reason: reason, severity: .warning)
return [issue]
}
struct LintableTarget: Equatable, Hashable {
let platform: Platform
let product: Product

6
Sources/TuistGenerator/Models/Target.swift
View File

@ -63,10 +63,16 @@ public class Target: Equatable {
self.dependencies = dependencies
}
/// Target can be included in the link phase of other targets
func isLinkable() -> Bool {
return [.dynamicLibrary, .staticLibrary, .framework, .staticFramework].contains(product)
}
/// Target can link staitc products (e.g. an app can link a staticLibrary)
func canLinkStaticProducts() -> Bool {
return [.framework, .app, .unitTests, .uiTests].contains(product)
}
/// Returns the product name including the extension.
var productNameWithExtension: String {
switch product {

2
Tests/TuistGeneratorTests/Graph/TestData/Graph+TestData.swift
View File

@ -44,7 +44,7 @@ extension Graph {
/// Note: For the purposes of testing, to reduce complexity of resolving dependencies
/// 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],
static func create(projects: [Project] = [],
dependencies: [(project: Project, target: Target, dependencies: [Target])]) -> Graph {
let targetNodes = createTargetNodes(dependencies: dependencies)

52
Tests/TuistGeneratorTests/Linter/GraphLinterTests.swift
View File

@ -82,6 +82,58 @@ final class GraphLinterTests: XCTestCase {
XCTAssertTrue(result.contains(LintingIssue(reason: "Target staticFramework has been linked against AppTarget and frameworkA, it is a static product so may introduce unwanted side effects.", severity: .warning)))
}
func test_lint_when_staticFramework_depends_on_static_products() throws {
// Given
let appTarget = Target.test(name: "AppTarget", product: .app)
let staticFrameworkA = Target.test(name: "staticFrameworkA", product: .staticFramework)
let staticFrameworkB = Target.test(name: "staticFrameworkB", product: .staticFramework)
let staticLibrary = Target.test(name: "staticLibrary", product: .staticLibrary)
let app = Project.test(path: "/tmp/app", name: "App", targets: [appTarget])
let frameworks = Project.test(path: "/tmp/staticframework",
name: "projectStaticFramework",
targets: [staticFrameworkA, staticFrameworkB, staticLibrary])
let graph = Graph.create(dependencies: [
(project: app, target: appTarget, dependencies: [staticFrameworkA, staticFrameworkB, staticLibrary]),
(project: frameworks, target: staticFrameworkA, dependencies: [staticFrameworkB]),
(project: frameworks, target: staticFrameworkB, dependencies: [staticLibrary]),
(project: frameworks, target: staticLibrary, dependencies: []),
])
// When
let result = subject.lint(graph: graph)
// Then
XCTAssertTrue(result.isEmpty)
}
func test_lint_when_staticLibrary_depends_on_static_products() throws {
// Given
let appTarget = Target.test(name: "AppTarget", product: .app)
let staticLibraryA = Target.test(name: "staticLibraryA", product: .staticLibrary)
let staticLibraryB = Target.test(name: "staticLibraryB", product: .staticLibrary)
let staticFramework = Target.test(name: "staticFramework", product: .staticFramework)
let app = Project.test(path: "/tmp/app", name: "App", targets: [appTarget])
let frameworks = Project.test(path: "/tmp/staticframework",
name: "projectStaticFramework",
targets: [staticLibraryA, staticLibraryB, staticFramework])
let graph = Graph.create(dependencies: [
(project: app, target: appTarget, dependencies: [staticLibraryA, staticLibraryB, staticFramework]),
(project: frameworks, target: staticLibraryA, dependencies: [staticLibraryB]),
(project: frameworks, target: staticLibraryB, dependencies: [staticFramework]),
(project: frameworks, target: staticFramework, dependencies: []),
])
// When
let result = subject.lint(graph: graph)
// Then
XCTAssertTrue(result.isEmpty)
}
func test_lint_frameworkDependsOnBundle() throws {
// Given
let bundle = Target.empty(name: "bundle", product: .bundle)

11
fixtures/README.md
View File

@ -133,7 +133,7 @@ Dependencies:
## ios_app_with_static_frameworks
Same as `ios_app_with_static_libraries` except using static frameworks instead of libraries.
This fixture contains an application that depends on static frameworks, both directly and transitively.
```
Workspace:
@ -141,16 +141,21 @@ Workspace:
- MainApp (iOS app)
- MainAppTests (iOS unit tests)
- A:
- A (static library iOS)
- A (static framework iOS)
- ATests (iOS unit tests)
- B:
- B (static library iOS)
- B (static framework iOS)
- BTests (iOS unit tests)
- C:
- C (static framework iOS)
- CTests (iOS unit tests)
```
Dependencies:
- App -> A
- App -> C
- A -> B
- A -> C
## ios_app_with_tests

1
fixtures/ios_app_with_static_frameworks/Modules/A/Project.swift
View File

@ -10,6 +10,7 @@ let project = Project(name: "A",
sources: "Sources/**",
dependencies: [
.project(target: "B", path: "../B"),
.project(target: "C", path: "../C"),
]),
Target(name: "ATests",
platform: .iOS,

3
fixtures/ios_app_with_static_frameworks/Modules/A/Sources/A.swift
View File

@ -1,4 +1,5 @@
import B
import C
public class A {
public static let value: String = "aValue"
@ -6,5 +7,7 @@ public class A {
public static func printFromA() {
print("print from A")
B.printFromB()
C.printFromC()
print("---")
}
}

26
fixtures/ios_app_with_static_frameworks/Modules/C/Info.plist Executable file
View File

@ -0,0 +1,26 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>CFBundleDevelopmentRegion</key>
<string>$(DEVELOPMENT_LANGUAGE)</string>
<key>CFBundleExecutable</key>
<string>$(EXECUTABLE_NAME)</string>
<key>CFBundleIdentifier</key>
<string>$(PRODUCT_BUNDLE_IDENTIFIER)</string>
<key>CFBundleInfoDictionaryVersion</key>
<string>6.0</string>
<key>CFBundleName</key>
<string>$(PRODUCT_NAME)</string>
<key>CFBundlePackageType</key>
<string>FMWK</string>
<key>CFBundleShortVersionString</key>
<string>1.0</string>
<key>CFBundleVersion</key>
<string>$(CURRENT_PROJECT_VERSION)</string>
<key>NSHumanReadableCopyright</key>
<string>Copyright ©. All rights reserved.</string>
<key>NSPrincipalClass</key>
<string></string>
</dict>
</plist>

24
fixtures/ios_app_with_static_frameworks/Modules/C/Project.swift Executable file
View File

@ -0,0 +1,24 @@
import ProjectDescription
let project = Project(name: "C",
targets: [
Target(name: "C",
platform: .iOS,
product: .staticFramework,
bundleId: "io.tuist.C",
infoPlist: "Info.plist",
sources: "Sources/**",
dependencies: [
/* Target dependencies can be defined here */
/* .framework(path: "framework") */
]),
Target(name: "CTests",
platform: .iOS,
product: .unitTests,
bundleId: "io.tuist.CTests",
infoPlist: "Tests.plist",
sources: "Tests/**",
dependencies: [
.target(name: "C"),
]),
])

9
fixtures/ios_app_with_static_frameworks/Modules/C/Sources/C.swift Executable file
View File

@ -0,0 +1,9 @@
import Foundation
public class C {
public static let value: String = "cValue"
public static func printFromC() {
print("print from C")
}
}

24
fixtures/ios_app_with_static_frameworks/Modules/C/Tests.plist Executable file
View File

@ -0,0 +1,24 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>CFBundleDevelopmentRegion</key>
<string>$(DEVELOPMENT_LANGUAGE)</string>
<key>CFBundleExecutable</key>
<string>$(EXECUTABLE_NAME)</string>
<key>CFBundleIdentifier</key>
<string>$(PRODUCT_BUNDLE_IDENTIFIER)</string>
<key>CFBundleInfoDictionaryVersion</key>
<string>6.0</string>
<key>CFBundleName</key>
<string>$(PRODUCT_NAME)</string>
<key>CFBundlePackageType</key>
<string>BNDL</string>
<key>CFBundleShortVersionString</key>
<string>1.0</string>
<key>CFBundleVersion</key>
<string>1</string>
<key>NSHumanReadableCopyright</key>
<string>Copyright ©. All rights reserved.</string>
</dict>
</plist>

10
fixtures/ios_app_with_static_frameworks/Modules/C/Tests/CTests.swift Executable file
View File

@ -0,0 +1,10 @@
import Foundation
import XCTest
@testable import C
final class CTests: XCTestCase {
func test_value() {
XCTAssertEqual(C.value, "cValue")
}
}

1
fixtures/ios_app_with_static_frameworks/Project.swift
View File

@ -10,6 +10,7 @@ let project = Project(name: "iOSAppWithTransistiveStaticFrameworks",
sources: "Sources/**",
dependencies: [
.project(target: "A", path: "Modules/A"),
.project(target: "C", path: "Modules/C"),
]),
Target(name: "AppTests",
platform: .iOS,

2
fixtures/ios_app_with_static_frameworks/Sources/AppDelegate.swift
View File

@ -1,4 +1,5 @@
import A
import C
import UIKit
@UIApplicationMain
@ -13,6 +14,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
window?.makeKeyAndVisible()
A.printFromA()
C.printFromC()
return true
}