From c9547a7ef7934e2e172d3daf81e776293915df5e Mon Sep 17 00:00:00 2001 From: mustiikhalil <26250654+mustiikhalil@users.noreply.github.com> Date: Sat, 16 Mar 2024 15:26:55 +0100 Subject: [PATCH] Adds FilesLinter that makes sure all required files are available Creates a file linter that check if all non-glob Files within the targets are available within the generated targets and project Moves File validation into the TargetLinter as discussed --- .../Graph/ModelExtensions/Target+Core.swift | 9 +++- .../TuistGenerator/Linter/TargetLinter.swift | 46 ++++++++++++++++++ .../TuistGraph/Models/SourceFileGlob.swift | 9 +++- Sources/TuistGraph/Models/Target.swift | 11 ++++- .../CopyFileElement+ManifestMapper.swift | 28 ++++++----- .../CopyFilesAction+ManifestMapper.swift | 15 ++++-- .../FileElement+ManifestMapper.swift | 3 -- .../ResourceFileElement+ManifestMapper.swift | 27 +++++------ .../Target+ManifestMapper.swift | 47 ++++++++++++++----- .../Extensions/AbsolutePath+Extras.swift | 2 +- 10 files changed, 144 insertions(+), 53 deletions(-) diff --git a/Sources/TuistCore/Graph/ModelExtensions/Target+Core.swift b/Sources/TuistCore/Graph/ModelExtensions/Target+Core.swift index 4f36d8d1027..3f0a149336b 100644 --- a/Sources/TuistCore/Graph/ModelExtensions/Target+Core.swift +++ b/Sources/TuistCore/Graph/ModelExtensions/Target+Core.swift @@ -52,11 +52,16 @@ extension Target { /// This method unfolds the source file globs subtracting the paths that are excluded and ignoring /// the files that don't have a supported source extension. /// - Parameter sources: List of source file glob to be unfolded. - public static func sources(targetName: String, sources: [SourceFileGlob]) throws -> [TuistGraph.SourceFile] { + public static func sources(targetName: String, sources: [SourceFileGlob]) throws -> (sourceFiles: [TuistGraph.SourceFile], paths: [AbsolutePath]) { var sourceFiles: [AbsolutePath: TuistGraph.SourceFile] = [:] var invalidGlobs: [InvalidGlob] = [] + var noneGlobPaths: [AbsolutePath] = [] for source in sources { + if !source.glob.isGlobComponent { + noneGlobPaths.append(source.path) + } + let sourcePath = try AbsolutePath(validating: source.glob) let base = try AbsolutePath(validating: sourcePath.dirname) @@ -98,6 +103,6 @@ extension Target { throw TargetError.invalidSourcesGlob(targetName: targetName, invalidGlobs: invalidGlobs) } - return Array(sourceFiles.values) + return (Array(sourceFiles.values), noneGlobPaths) } } diff --git a/Sources/TuistGenerator/Linter/TargetLinter.swift b/Sources/TuistGenerator/Linter/TargetLinter.swift index df232d96337..7c31a5a86f7 100644 --- a/Sources/TuistGenerator/Linter/TargetLinter.swift +++ b/Sources/TuistGenerator/Linter/TargetLinter.swift @@ -40,6 +40,7 @@ class TargetLinter: TargetLinting { issues.append(contentsOf: validateCoreDataModelsExist(target: target)) issues.append(contentsOf: validateCoreDataModelVersionsExist(target: target)) issues.append(contentsOf: lintMergeableLibrariesOnlyAppliesToDynamicTargets(target: target)) + issues.append(contentsOf: lintAllRequiredFilesAvailable(target: target)) for script in target.scripts { issues.append(contentsOf: targetScriptLinter.lint(script)) } @@ -292,6 +293,51 @@ class TargetLinter: TargetLinting { } return [] } + + private func lintAllRequiredFilesAvailable(target: Target) -> [LintingIssue] { + var lintingIssues: [LintingIssue] = [] + + for path in target.sourcesPaths { + if target.sources.isEmpty && !path.isOpaqueDirectory { + lintingIssues.append(LintingIssue(reason: "No files found at: \(path)", severity: .warning)) + continue + } + guard target.sources.first(where: { $0.path == path }) != nil else { + lintingIssues.append(LintingIssue(reason: "No files found at: \(path)", severity: .warning)) + continue + } + } + + if target.supportsResources { + for path in target.resourcesPaths { + if target.resources.resources.isEmpty { + lintingIssues.append(LintingIssue(reason: "No resources found at: \(path)", severity: .warning)) + continue + } + guard target.resources.resources.first(where: { $0.path == path }) != nil else { + lintingIssues.append(LintingIssue(reason: "No resources found at: \(path)", severity: .warning)) + continue + } + } + } + + if target.supportsResources { + let files = target.copyFiles.flatMap { $0.files } + + for path in target.copyFilesPaths { + if target.copyFiles.isEmpty { + lintingIssues.append(LintingIssue(reason: "No copy files found at: \(path)", severity: .warning)) + continue + } + guard files.first(where: { $0.path == path }) != nil else { + lintingIssues.append(LintingIssue(reason: "No copy files found at: \(path)", severity: .warning)) + continue + } + } + } + + return lintingIssues + } } extension TargetDependency { diff --git a/Sources/TuistGraph/Models/SourceFileGlob.swift b/Sources/TuistGraph/Models/SourceFileGlob.swift index 5c142f48117..b7ee68f6a4f 100644 --- a/Sources/TuistGraph/Models/SourceFileGlob.swift +++ b/Sources/TuistGraph/Models/SourceFileGlob.swift @@ -1,10 +1,14 @@ import Foundation +import TSCBasic /// A type that represents a list of source files defined by a glob. public struct SourceFileGlob: Equatable { /// Glob pattern to unfold all the source files. public let glob: String + /// Reference to the AbsolutePath + public let path: AbsolutePath + /// Glob pattern used for filtering out files. public let excluding: [String] @@ -25,13 +29,14 @@ public struct SourceFileGlob: Equatable { /// - codeGen: Source file code generation attribute. /// - compilationCondition: Condition for file compilation. public init( - glob: String, + glob: AbsolutePath, excluding: [String] = [], compilerFlags: String? = nil, codeGen: FileCodeGen? = nil, compilationCondition: PlatformCondition? = nil ) { - self.glob = glob + self.glob = glob.pathString + self.path = glob self.excluding = excluding self.compilerFlags = compilerFlags self.codeGen = codeGen diff --git a/Sources/TuistGraph/Models/Target.swift b/Sources/TuistGraph/Models/Target.swift index 3f13d5b6de7..a1c4a5fd037 100644 --- a/Sources/TuistGraph/Models/Target.swift +++ b/Sources/TuistGraph/Models/Target.swift @@ -12,7 +12,7 @@ public struct Target: Equatable, Hashable, Comparable, Codable { "playground", "rcproject", "mlpackage", ] public static let validFolderExtensions: [String] = [ - "framework", "bundle", "app", "xcassets", "appiconset", "scnassets", + "framework", "bundle", "app", "xcassets", "appiconset", "scnassets", "strings" ] // MARK: - Attributes @@ -31,8 +31,11 @@ public struct Target: Equatable, Hashable, Comparable, Codable { public var settings: Settings? public var dependencies: [TargetDependency] public var sources: [SourceFile] + public var sourcesPaths: [AbsolutePath] public var resources: ResourceFileElements + public var resourcesPaths: [AbsolutePath] public var copyFiles: [CopyFilesAction] + public var copyFilesPaths: [AbsolutePath] public var headers: Headers? public var coreDataModels: [CoreDataModel] public var scripts: [TargetScript] @@ -60,8 +63,11 @@ public struct Target: Equatable, Hashable, Comparable, Codable { entitlements: Entitlements? = nil, settings: Settings? = nil, sources: [SourceFile] = [], + sourcesPaths: [AbsolutePath] = [], resources: ResourceFileElements = .init([]), + resourcesPaths: [AbsolutePath] = [], copyFiles: [CopyFilesAction] = [], + copyFilesPaths: [AbsolutePath] = [], headers: Headers? = nil, coreDataModels: [CoreDataModel] = [], scripts: [TargetScript] = [], @@ -87,8 +93,11 @@ public struct Target: Equatable, Hashable, Comparable, Codable { self.entitlements = entitlements self.settings = settings self.sources = sources + self.sourcesPaths = sourcesPaths self.resources = resources + self.resourcesPaths = resourcesPaths self.copyFiles = copyFiles + self.copyFilesPaths = copyFilesPaths self.headers = headers self.coreDataModels = coreDataModels self.scripts = scripts diff --git a/Sources/TuistLoader/Models+ManifestMappers/CopyFileElement+ManifestMapper.swift b/Sources/TuistLoader/Models+ManifestMappers/CopyFileElement+ManifestMapper.swift index d6e1edb5059..bdb36cb780b 100644 --- a/Sources/TuistLoader/Models+ManifestMappers/CopyFileElement+ManifestMapper.swift +++ b/Sources/TuistLoader/Models+ManifestMappers/CopyFileElement+ManifestMapper.swift @@ -15,9 +15,9 @@ extension TuistGraph.CopyFileElement { manifest: ProjectDescription.CopyFileElement, generatorPaths: GeneratorPaths, includeFiles: @escaping (AbsolutePath) -> Bool = { _ in true } - ) throws -> [TuistGraph.CopyFileElement] { - func globFiles(_ path: AbsolutePath) throws -> [AbsolutePath] { - if FileHandler.shared.exists(path), !FileHandler.shared.isFolder(path) { return [path] } + ) throws -> (files: [TuistGraph.CopyFileElement], path: AbsolutePath) { + func globFiles(_ path: AbsolutePath) throws -> (files: [AbsolutePath], path: AbsolutePath) { + if FileHandler.shared.exists(path), !FileHandler.shared.isFolder(path) { return ([path], path) } let files = try FileHandler.shared.throwingGlob(AbsolutePath.root, glob: String(path.pathString.dropFirst())) .filter(includeFiles) @@ -26,37 +26,41 @@ extension TuistGraph.CopyFileElement { if FileHandler.shared.isFolder(path) { logger.warning("'\(path.pathString)' is a directory, try using: '\(path.pathString)/**' to list its files") } else { - // FIXME: This should be done in a linter. - logger.warning("No files found at: \(path.pathString)") + if !path.pathString.isGlobComponent { + // FIXME: This should be done in a linter. + logger.warning("No files found at: \(path.pathString)") + } } } - return files + return (files, path) } - func folderReferences(_ path: AbsolutePath) -> [AbsolutePath] { + func folderReferences(_ path: AbsolutePath) -> (files: [AbsolutePath], path: AbsolutePath) { guard FileHandler.shared.exists(path) else { // FIXME: This should be done in a linter. logger.warning("\(path.pathString) does not exist") - return [] + return ([], path) } guard FileHandler.shared.isFolder(path) else { // FIXME: This should be done in a linter. logger.warning("\(path.pathString) is not a directory - folder reference paths need to point to directories") - return [] + return ([], path) } - return [path] + return ([path], path) } switch manifest { case let .glob(pattern: pattern, condition: condition): let resolvedPath = try generatorPaths.resolve(path: pattern) - return try globFiles(resolvedPath).map { .file(path: $0, condition: condition?.asGraphCondition) } + let copyFiles = try globFiles(resolvedPath) + return (copyFiles.files.map { .file(path: $0, condition: condition?.asGraphCondition) }, copyFiles.path) case let .folderReference(path: folderReferencePath, condition: condition): let resolvedPath = try generatorPaths.resolve(path: folderReferencePath) - return folderReferences(resolvedPath).map { .folderReference(path: $0, condition: condition?.asGraphCondition) } + let copyFiles = folderReferences(resolvedPath) + return (copyFiles.files.map { .folderReference(path: $0, condition: condition?.asGraphCondition) }, copyFiles.path) } } } diff --git a/Sources/TuistLoader/Models+ManifestMappers/CopyFilesAction+ManifestMapper.swift b/Sources/TuistLoader/Models+ManifestMappers/CopyFilesAction+ManifestMapper.swift index 512c42067fd..3c1f5ae515f 100644 --- a/Sources/TuistLoader/Models+ManifestMappers/CopyFilesAction+ManifestMapper.swift +++ b/Sources/TuistLoader/Models+ManifestMappers/CopyFilesAction+ManifestMapper.swift @@ -24,10 +24,12 @@ extension TuistGraph.CopyFilesAction { /// - Parameters: /// - manifest: Manifest representation of platform model. /// - generatorPaths: Generator paths. - static func from(manifest: ProjectDescription.CopyFilesAction, generatorPaths: GeneratorPaths) throws -> TuistGraph - .CopyFilesAction + static func from(manifest: ProjectDescription.CopyFilesAction, generatorPaths: GeneratorPaths) throws -> (copyAction: TuistGraph + .CopyFilesAction, paths: [AbsolutePath]) { var invalidResourceGlobs: [InvalidGlob] = [] + var noneGlobFiles: [AbsolutePath] = [] + let files: [TuistGraph.CopyFileElement] = try manifest.files.flatMap { manifest -> [TuistGraph.CopyFileElement] in do { let files = try TuistGraph.CopyFileElement.from( @@ -35,7 +37,10 @@ extension TuistGraph.CopyFilesAction { generatorPaths: generatorPaths, includeFiles: { TuistGraph.Target.isResource(path: $0) } ) - return files.cleanPackages() + if !files.path.pathString.isGlobComponent { + noneGlobFiles.append(files.path) + } + return files.files.cleanPackages() } catch let GlobError.nonExistentDirectory(invalidGlob) { invalidResourceGlobs.append(invalidGlob) return [] @@ -46,12 +51,12 @@ extension TuistGraph.CopyFilesAction { throw CopyFilesManifestMapperError.invalidResourcesGlob(actionName: manifest.name, invalidGlobs: invalidResourceGlobs) } - return TuistGraph.CopyFilesAction( + return (TuistGraph.CopyFilesAction( name: manifest.name, destination: TuistGraph.CopyFilesAction.Destination.from(manifest: manifest.destination), subpath: manifest.subpath, files: files - ) + ), noneGlobFiles) } } diff --git a/Sources/TuistLoader/Models+ManifestMappers/FileElement+ManifestMapper.swift b/Sources/TuistLoader/Models+ManifestMappers/FileElement+ManifestMapper.swift index 2501c03c573..e477d07bcac 100644 --- a/Sources/TuistLoader/Models+ManifestMappers/FileElement+ManifestMapper.swift +++ b/Sources/TuistLoader/Models+ManifestMappers/FileElement+ManifestMapper.swift @@ -25,9 +25,6 @@ extension TuistGraph.FileElement { if files.isEmpty { if FileHandler.shared.isFolder(path) { logger.warning("'\(path.pathString)' is a directory, try using: '\(path.pathString)/**' to list its files") - } else { - // FIXME: This should be done in a linter. - logger.warning("No files found at: \(path.pathString)") } } diff --git a/Sources/TuistLoader/Models+ManifestMappers/ResourceFileElement+ManifestMapper.swift b/Sources/TuistLoader/Models+ManifestMappers/ResourceFileElement+ManifestMapper.swift index 58c678f5f77..f92872b8ade 100644 --- a/Sources/TuistLoader/Models+ManifestMappers/ResourceFileElement+ManifestMapper.swift +++ b/Sources/TuistLoader/Models+ManifestMappers/ResourceFileElement+ManifestMapper.swift @@ -15,8 +15,8 @@ extension TuistGraph.ResourceFileElement { manifest: ProjectDescription.ResourceFileElement, generatorPaths: GeneratorPaths, includeFiles: @escaping (AbsolutePath) -> Bool = { _ in true } - ) throws -> [TuistGraph.ResourceFileElement] { - func globFiles(_ path: AbsolutePath, excluding: [String]) throws -> [AbsolutePath] { + ) throws -> (resources: [TuistGraph.ResourceFileElement], path: AbsolutePath) { + func globFiles(_ path: AbsolutePath, excluding: [String]) throws -> (resources: [AbsolutePath], path: AbsolutePath) { var excluded: Set = [] for path in excluding { let absolute = try AbsolutePath(validating: path) @@ -33,47 +33,46 @@ extension TuistGraph.ResourceFileElement { if files.isEmpty { if FileHandler.shared.isFolder(path) { logger.warning("'\(path.pathString)' is a directory, try using: '\(path.pathString)/**' to list its files") - } else { - // FIXME: This should be done in a linter. - logger.warning("No files found at: \(path.pathString)") } } - return files + return (files, path) } - func folderReferences(_ path: AbsolutePath) -> [AbsolutePath] { + func folderReferences(_ path: AbsolutePath) -> (resources: [AbsolutePath], path: AbsolutePath) { guard FileHandler.shared.exists(path) else { // FIXME: This should be done in a linter. logger.warning("\(path.pathString) does not exist") - return [] + return ([], path) } guard FileHandler.shared.isFolder(path) else { // FIXME: This should be done in a linter. logger.warning("\(path.pathString) is not a directory - folder reference paths need to point to directories") - return [] + return ([], path) } - return [path] + return ([path], path) } switch manifest { case let .glob(pattern, excluding, tags, condition): let resolvedPath = try generatorPaths.resolve(path: pattern) let excluding: [String] = try excluding.compactMap { try generatorPaths.resolve(path: $0).pathString } - return try globFiles(resolvedPath, excluding: excluding).map { ResourceFileElement.file( + let files = try globFiles(resolvedPath, excluding: excluding) + return (files.resources.map { ResourceFileElement.file( path: $0, tags: tags, inclusionCondition: condition?.asGraphCondition - ) } + ) }, files.path) case let .folderReference(folderReferencePath, tags, condition): let resolvedPath = try generatorPaths.resolve(path: folderReferencePath) - return folderReferences(resolvedPath).map { ResourceFileElement.folderReference( + let files = folderReferences(resolvedPath) + return (files.resources.map { ResourceFileElement.folderReference( path: $0, tags: tags, inclusionCondition: condition?.asGraphCondition - ) } + ) }, files.path) } } } diff --git a/Sources/TuistLoader/Models+ManifestMappers/Target+ManifestMapper.swift b/Sources/TuistLoader/Models+ManifestMappers/Target+ManifestMapper.swift index a71558783a2..6018ab76f3b 100644 --- a/Sources/TuistLoader/Models+ManifestMappers/Target+ManifestMapper.swift +++ b/Sources/TuistLoader/Models+ManifestMappers/Target+ManifestMapper.swift @@ -54,13 +54,13 @@ extension TuistGraph.Target { let settings = try manifest.settings.map { try TuistGraph.Settings.from(manifest: $0, generatorPaths: generatorPaths) } let mergedBinaryType = try TuistGraph.MergedBinaryType.from(manifest: manifest.mergedBinaryType) - let (sources, sourcesPlaygrounds) = try sourcesAndPlaygrounds( + let (sources, sourcesPlaygrounds, sourcesPaths) = try sourcesAndPlaygrounds( manifest: manifest, targetName: name, generatorPaths: generatorPaths ) - let (resources, resourcesPlaygrounds, resourcesCoreDatas, invalidResourceGlobs) = try resourcesAndOthers( + let (resources, resourcesPlaygrounds, resourcesCoreDatas, invalidResourceGlobs, resourcesPaths) = try resourcesAndOthers( manifest: manifest, generatorPaths: generatorPaths ) @@ -69,9 +69,7 @@ extension TuistGraph.Target { throw TargetManifestMapperError.invalidResourcesGlob(targetName: name, invalidGlobs: invalidResourceGlobs) } - let copyFiles = try (manifest.copyFiles ?? []).map { - try TuistGraph.CopyFilesAction.from(manifest: $0, generatorPaths: generatorPaths) - } + let copyFiles = try copyFiles(manifest: manifest, generatorPaths: generatorPaths) let headers = try manifest.headers.map { try TuistGraph.Headers.from( manifest: $0, @@ -110,8 +108,11 @@ extension TuistGraph.Target { entitlements: entitlements, settings: settings, sources: sources, + sourcesPaths: sourcesPaths, resources: resources, - copyFiles: copyFiles, + resourcesPaths: resourcesPaths, + copyFiles: copyFiles.copyActions, + copyFilesPaths: copyFiles.paths, headers: headers, coreDataModels: coreDataModels, scripts: scripts, @@ -137,7 +138,8 @@ extension TuistGraph.Target { resources: TuistGraph.ResourceFileElements, playgrounds: [AbsolutePath], coreDataModels: [AbsolutePath], - invalidResourceGlobs: [InvalidGlob] + invalidResourceGlobs: [InvalidGlob], + paths: [AbsolutePath] ) { let resourceFilter = { (path: AbsolutePath) -> Bool in TuistGraph.Target.isResource(path: path) @@ -156,14 +158,19 @@ extension TuistGraph.Target { var filteredResources: TuistGraph.ResourceFileElements = .init([], privacyManifest: privacyManifest) var playgrounds: Set = [] var coreDataModels: Set = [] + var noneGlobPaths: Set = [] let allResources = try (manifest.resources?.resources ?? []).flatMap { manifest -> [TuistGraph.ResourceFileElement] in do { - return try TuistGraph.ResourceFileElement.from( + let resourcesPath = try TuistGraph.ResourceFileElement.from( manifest: manifest, generatorPaths: generatorPaths, includeFiles: resourceFilter ) + if !noneGlobPaths.contains(resourcesPath.path) && !resourcesPath.path.pathString.isGlobComponent { + noneGlobPaths.insert(resourcesPath.path) + } + return resourcesPath.resources } catch let GlobError.nonExistentDirectory(invalidGlob) { invalidResourceGlobs.append(invalidGlob) return [] @@ -188,7 +195,8 @@ extension TuistGraph.Target { resources: filteredResources, playgrounds: Array(playgrounds), coreDataModels: Array(coreDataModels), - invalidResourceGlobs: invalidResourceGlobs + invalidResourceGlobs: invalidResourceGlobs, + paths: Array(noneGlobPaths) ) } @@ -196,13 +204,13 @@ extension TuistGraph.Target { manifest: ProjectDescription.Target, targetName: String, generatorPaths: GeneratorPaths - ) throws -> (sources: [TuistGraph.SourceFile], playgrounds: [AbsolutePath]) { + ) throws -> (sources: [TuistGraph.SourceFile], playgrounds: [AbsolutePath], paths: [AbsolutePath]) { var sourcesWithoutPlaygrounds: [TuistGraph.SourceFile] = [] var playgrounds: Set = [] // Sources let allSources = try TuistGraph.Target.sources(targetName: targetName, sources: manifest.sources?.globs.map { glob in - let globPath = try generatorPaths.resolve(path: glob.glob).pathString + let globPath = try generatorPaths.resolve(path: glob.glob) let excluding: [String] = try glob.excluding.compactMap { try generatorPaths.resolve(path: $0).pathString } let mappedCodeGen = glob.codeGen.map(TuistGraph.FileCodeGen.from) return TuistGraph.SourceFileGlob( @@ -214,7 +222,7 @@ extension TuistGraph.Target { ) } ?? []) - for sourceFile in allSources { + for sourceFile in allSources.sourceFiles { if sourceFile.path.extension == "playground" { playgrounds.insert(sourceFile.path) } else { @@ -222,7 +230,20 @@ extension TuistGraph.Target { } } - return (sources: sourcesWithoutPlaygrounds, playgrounds: Array(playgrounds)) + return (sources: sourcesWithoutPlaygrounds, playgrounds: Array(playgrounds), paths: allSources.paths) + } + + fileprivate static func copyFiles( + manifest: ProjectDescription.Target, + generatorPaths: GeneratorPaths + ) throws -> (copyActions: [TuistGraph.CopyFilesAction], paths: [AbsolutePath]) { + var nonGlobFiles: [AbsolutePath] = [] + let copyFiles = try (manifest.copyFiles ?? []).map { + let files = try TuistGraph.CopyFilesAction.from(manifest: $0, generatorPaths: generatorPaths) + nonGlobFiles.append(contentsOf: files.paths) + return files.copyAction + } + return (copyFiles, nonGlobFiles) } // swiftlint:enable function_body_length } diff --git a/Sources/TuistSupport/Extensions/AbsolutePath+Extras.swift b/Sources/TuistSupport/Extensions/AbsolutePath+Extras.swift index 7283fde0973..0965fa54857 100644 --- a/Sources/TuistSupport/Extensions/AbsolutePath+Extras.swift +++ b/Sources/TuistSupport/Extensions/AbsolutePath+Extras.swift @@ -151,7 +151,7 @@ extension AbsolutePath: ExpressibleByStringLiteral { } extension String { - var isGlobComponent: Bool { + public var isGlobComponent: Bool { let globCharacters = CharacterSet(charactersIn: "*{}") return rangeOfCharacter(from: globCharacters) != nil }