Skip to content

Commit 9d40d17

Browse files
tomerdtomerd
authored
dont emit resources or excludes warnings for dependencies (#3857)
motivation: SwiftPM emits warning on invalid excludes and missing resources definitions for dependencies which is not actionable and noisy changes: * in TargetSourcesBuilder check the package type before emiting resources related warnings * add tests
1 parent 1fa303d commit 9d40d17

File tree

2 files changed

+168
-45
lines changed

2 files changed

+168
-45
lines changed

‎Sources/PackageLoading/TargetSourcesBuilder.swift

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ public struct TargetSourcesBuilder {
106106
self.declaredSources = declaredSources?.spm_uniqueElements()
107107

108108
self.excludedPaths.forEach { exclude in
109-
if let message = validTargetPath(at: exclude) {
109+
if let message = validTargetPath(at: exclude), self.packageKind.emitAuthorWarnings {
110110
let warning = "Invalid Exclude '\(exclude)': \(message)."
111111
self.observabilityScope.emit(warning: warning)
112112
}
@@ -168,7 +168,7 @@ public struct TargetSourcesBuilder {
168168
var others: [AbsolutePath] = []
169169
if toolsVersion >= .v5_3 {
170170
let filesWithNoRules = pathToRule.filter { $0.value.rule == .none }
171-
if !filesWithNoRules.isEmpty {
171+
if !filesWithNoRules.isEmpty, self.packageKind.emitAuthorWarnings {
172172
var warning = "found \(filesWithNoRules.count) file(s) which are unhandled; explicitly declare them as resources or exclude from the target\n"
173173
for (file, _) in filesWithNoRules {
174174
warning += " " + file.pathString + "\n"
@@ -369,7 +369,7 @@ public struct TargetSourcesBuilder {
369369
private func diagnoseInvalidResource(in resources: [TargetDescription.Resource]) {
370370
resources.forEach { resource in
371371
let resourcePath = self.targetPath.appending(RelativePath(resource.path))
372-
if let message = validTargetPath(at: resourcePath) {
372+
if let message = validTargetPath(at: resourcePath), self.packageKind.emitAuthorWarnings {
373373
let warning = "Invalid Resource '\(resource.path)': \(message)."
374374
self.observabilityScope.emit(warning: warning)
375375
}
@@ -708,3 +708,14 @@ extension ObservabilityMetadata {
708708
typealias Value = String
709709
}
710710
}
711+
712+
fileprivate extension PackageReference.Kind {
713+
var emitAuthorWarnings: Bool {
714+
switch self {
715+
case .remoteSourceControl, .registry:
716+
return false
717+
default:
718+
return true
719+
}
720+
}
721+
}

‎Tests/PackageLoadingTests/TargetSourcesBuilderTests.swift

Lines changed: 154 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
*/
1010

1111
import Basics
12+
import Foundation
1213
import PackageModel
1314
import PackageLoading
1415
import SPMTestSupport
@@ -655,31 +656,56 @@ class TargetSourcesBuilderTests: XCTestCase {
655656
"/Bar.swift"
656657
])
657658

658-
let observability = ObservabilitySystem.makeForTesting()
659659

660-
let builder = TargetSourcesBuilder(
661-
packageIdentity: .plain("test"),
662-
packageKind: .root(.init("/test")),
663-
packagePath: .init("/test"),
664-
target: target,
665-
path: .root,
666-
defaultLocalization: nil,
667-
toolsVersion: .v5,
668-
fileSystem: fs,
669-
observabilityScope: observability.topScope
670-
)
660+
do {
661+
let observability = ObservabilitySystem.makeForTesting()
662+
663+
let builder = TargetSourcesBuilder(
664+
packageIdentity: .plain("test"),
665+
packageKind: .root(.init("/test")),
666+
packagePath: .init("/test"),
667+
target: target,
668+
path: .root,
669+
defaultLocalization: nil,
670+
toolsVersion: .v5,
671+
fileSystem: fs,
672+
observabilityScope: observability.topScope
673+
)
674+
_ = try builder.run()
671675

672-
testDiagnostics(observability.diagnostics) { result in
673-
var diagnosticsFound = [Basics.Diagnostic?]()
674-
diagnosticsFound.append(result.checkUnordered(diagnostic: "Invalid Exclude '/fileOutsideRoot.py': File not found.", severity: .warning))
675-
diagnosticsFound.append(result.checkUnordered(diagnostic: "Invalid Exclude '/fakeDir': File not found.", severity: .warning))
676+
testDiagnostics(observability.diagnostics) { result in
677+
var diagnosticsFound = [Basics.Diagnostic?]()
678+
diagnosticsFound.append(result.checkUnordered(diagnostic: "Invalid Exclude '/fileOutsideRoot.py': File not found.", severity: .warning))
679+
diagnosticsFound.append(result.checkUnordered(diagnostic: "Invalid Exclude '/fakeDir': File not found.", severity: .warning))
676680

677-
for diagnostic in diagnosticsFound {
678-
XCTAssertEqual(diagnostic?.metadata?.packageIdentity, builder.packageIdentity)
679-
XCTAssertEqual(diagnostic?.metadata?.packageKind, builder.packageKind)
680-
XCTAssertEqual(diagnostic?.metadata?.targetName, target.name)
681+
for diagnostic in diagnosticsFound {
682+
XCTAssertEqual(diagnostic?.metadata?.packageIdentity, builder.packageIdentity)
683+
XCTAssertEqual(diagnostic?.metadata?.packageKind, builder.packageKind)
684+
XCTAssertEqual(diagnostic?.metadata?.targetName, target.name)
685+
}
681686
}
682687
}
688+
689+
// should not emit for "remote" packages
690+
691+
do {
692+
let observability = ObservabilitySystem.makeForTesting()
693+
694+
let builder = TargetSourcesBuilder(
695+
packageIdentity: .plain("test"),
696+
packageKind: .remoteSourceControl(URL(string: "https://some.where/foo/bar")!),
697+
packagePath: .init("/test"),
698+
target: target,
699+
path: .root,
700+
defaultLocalization: nil,
701+
toolsVersion: .v5,
702+
fileSystem: fs,
703+
observabilityScope: observability.topScope
704+
)
705+
_ = try builder.run()
706+
707+
XCTAssertNoDiagnostics(observability.diagnostics)
708+
}
683709
}
684710

685711
func testMissingResource() throws {
@@ -700,33 +726,57 @@ class TargetSourcesBuilderTests: XCTestCase {
700726
"/Bar.swift"
701727
])
702728

703-
let observability = ObservabilitySystem.makeForTesting()
704-
705-
let builder = TargetSourcesBuilder(
706-
packageIdentity: .plain("test"),
707-
packageKind: .root(.root),
708-
packagePath: .root,
709-
target: target,
710-
path: .root,
711-
defaultLocalization: nil,
712-
toolsVersion: .v5,
713-
fileSystem: fs,
714-
observabilityScope: observability.topScope
715-
)
716-
_ = try builder.run()
729+
do {
730+
let observability = ObservabilitySystem.makeForTesting()
731+
732+
let builder = TargetSourcesBuilder(
733+
packageIdentity: .plain("test"),
734+
packageKind: .root(.root),
735+
packagePath: .root,
736+
target: target,
737+
path: .root,
738+
defaultLocalization: nil,
739+
toolsVersion: .v5,
740+
fileSystem: fs,
741+
observabilityScope: observability.topScope
742+
)
743+
_ = try builder.run()
717744

718-
testDiagnostics(observability.diagnostics) { result in
719-
var diagnosticsFound = [Basics.Diagnostic?]()
720-
diagnosticsFound.append(result.checkUnordered(diagnostic: "Invalid Resource '../../../Fake.txt': File not found.", severity: .warning))
721-
diagnosticsFound.append(result.checkUnordered(diagnostic: "Invalid Resource 'NotReal': File not found.", severity: .warning))
745+
testDiagnostics(observability.diagnostics) { result in
746+
var diagnosticsFound = [Basics.Diagnostic?]()
747+
diagnosticsFound.append(result.checkUnordered(diagnostic: "Invalid Resource '../../../Fake.txt': File not found.", severity: .warning))
748+
diagnosticsFound.append(result.checkUnordered(diagnostic: "Invalid Resource 'NotReal': File not found.", severity: .warning))
722749

723-
for diagnostic in diagnosticsFound {
724-
XCTAssertEqual(diagnostic?.metadata?.packageIdentity, builder.packageIdentity)
725-
XCTAssertEqual(diagnostic?.metadata?.packageKind, builder.packageKind)
726-
XCTAssertEqual(diagnostic?.metadata?.targetName, target.name)
750+
for diagnostic in diagnosticsFound {
751+
XCTAssertEqual(diagnostic?.metadata?.packageIdentity, builder.packageIdentity)
752+
XCTAssertEqual(diagnostic?.metadata?.packageKind, builder.packageKind)
753+
XCTAssertEqual(diagnostic?.metadata?.targetName, target.name)
754+
}
727755
}
728756
}
757+
758+
// should not emit for "remote" packages
759+
760+
do {
761+
let observability = ObservabilitySystem.makeForTesting()
762+
763+
let builder = TargetSourcesBuilder(
764+
packageIdentity: .plain("test"),
765+
packageKind: .remoteSourceControl(URL(string: "https://some.where/foo/bar")!),
766+
packagePath: .root,
767+
target: target,
768+
path: .root,
769+
defaultLocalization: nil,
770+
toolsVersion: .v5,
771+
fileSystem: fs,
772+
observabilityScope: observability.topScope
773+
)
774+
_ = try builder.run()
775+
776+
XCTAssertNoDiagnostics(observability.diagnostics)
777+
}
729778
}
779+
730780

731781
func testMissingSource() throws {
732782
let target = try TargetDescription(
@@ -814,6 +864,68 @@ class TargetSourcesBuilderTests: XCTestCase {
814864
}
815865
}
816866

867+
func testUnhandledResources() throws {
868+
let target = try TargetDescription(
869+
name: "Foo",
870+
path: nil,
871+
exclude: [],
872+
sources: ["File.swift"],
873+
resources: [],
874+
publicHeadersPath: nil,
875+
type: .regular
876+
)
877+
878+
let fs = InMemoryFileSystem()
879+
fs.createEmptyFiles(at: .root, files: [
880+
"/File.swift",
881+
"/foo.bar"
882+
])
883+
884+
do {
885+
let observability = ObservabilitySystem.makeForTesting()
886+
887+
let builder = TargetSourcesBuilder(
888+
packageIdentity: .plain("test"),
889+
packageKind: .root(.init("/test")),
890+
packagePath: .root,
891+
target: target,
892+
path: .root,
893+
defaultLocalization: nil,
894+
toolsVersion: .v5_5,
895+
fileSystem: fs,
896+
observabilityScope: observability.topScope
897+
)
898+
_ = try builder.run()
899+
900+
testDiagnostics(observability.diagnostics) { result in
901+
let diagnostic = result.check(diagnostic: "found 1 file(s) which are unhandled; explicitly declare them as resources or exclude from the target\n /foo.bar\n", severity: .warning)
902+
XCTAssertEqual(diagnostic?.metadata?.packageIdentity, builder.packageIdentity)
903+
XCTAssertEqual(diagnostic?.metadata?.targetName, target.name)
904+
}
905+
}
906+
907+
// should not emit for "remote" packages
908+
909+
do {
910+
let observability = ObservabilitySystem.makeForTesting()
911+
912+
let builder = TargetSourcesBuilder(
913+
packageIdentity: .plain("test"),
914+
packageKind: .remoteSourceControl(URL(string: "https://some.where/foo/bar")!),
915+
packagePath: .root,
916+
target: target,
917+
path: .root,
918+
defaultLocalization: nil,
919+
toolsVersion: .v5_5,
920+
fileSystem: fs,
921+
observabilityScope: observability.topScope
922+
)
923+
_ = try builder.run()
924+
925+
XCTAssertNoDiagnostics(observability.diagnostics)
926+
}
927+
}
928+
817929
func testDocCFilesDoNotCauseWarningOutsideXCBuild() throws {
818930
let target = try TargetDescription(
819931
name: "Foo",

0 commit comments

Comments
 (0)