From c3560e914d0f6778dc8ce850fe6cb517b749fded Mon Sep 17 00:00:00 2001 From: Chuck Grindel Date: Wed, 28 Jun 2023 15:19:09 -0600 Subject: [PATCH] fix: Gh425 add swift src info to index (#431) - Move `swift.FileInfo` to `swiftpkg.SwiftFileInfo`. - Update `swiftpkg.SwiftFileInfo` to check for `@objc` directive. - Add `ModulemapLabel` to `swift.Module`. For Swift source modules, this field is populated if a modulemap target is required (e.g., has `@objc` directives). - Add `modulemap_label` to `deps_indexes.new_module`. - Update `deps_indexes.labels_for_module` to check for `modulemap_label` on the module instead of assuming that all Swift targets that are a dependency for an Objc target should have one. Related to #425. --- .../firebase_example/swift_deps_index.json | 18 +++- .../internal/reslog/rule_resolution_test.go | 1 + gazelle/internal/swift/BUILD.bazel | 2 - gazelle/internal/swift/bazel_label.go | 9 ++ gazelle/internal/swift/bazel_label_test.go | 7 ++ gazelle/internal/swift/http_archive.go | 2 +- gazelle/internal/swift/module.go | 36 +++++++- gazelle/internal/swift/module_test.go | 2 +- gazelle/internal/swift/rules_from_srcs.go | 7 +- .../internal/swiftcfg/swift_config_test.go | 3 +- gazelle/internal/swiftpkg/BUILD.bazel | 2 + gazelle/internal/swiftpkg/package_info.go | 2 +- .../swift_file_info.go} | 71 ++++++++++----- .../swift_file_info_test.go} | 86 +++++++++++++++---- gazelle/internal/swiftpkg/target.go | 17 +++- swiftpkg/internal/deps_indexes.bzl | 22 ++++- swiftpkg/tests/deps_indexes_tests.bzl | 59 ++++++++++++- 17 files changed, 282 insertions(+), 64 deletions(-) rename gazelle/internal/{swift/file_info.go => swiftpkg/swift_file_info.go} (53%) rename gazelle/internal/{swift/file_info_test.go => swiftpkg/swift_file_info_test.go} (53%) diff --git a/examples/firebase_example/swift_deps_index.json b/examples/firebase_example/swift_deps_index.json index 896c5f4c5..fb12eb621 100644 --- a/examples/firebase_example/swift_deps_index.json +++ b/examples/firebase_example/swift_deps_index.json @@ -106,7 +106,7 @@ "name": "FirebaseAnalyticsWithoutAdIdSupportWrapper", "c99name": "FirebaseAnalyticsWithoutAdIdSupportWrapper", "src_type": "objc", - "label": "@swiftpkg_firebase_ios_sdk//:FirebaseAnalyticsWithoutAdIdSupportWrapper", + "label": "@swiftpkg_firebase_ios_sdk//:FirebaseAnalyticsWithoutAdIdSupportWrapper_FirebaseAnalyticsWithoutAdIdSupportWrapper", "package_identity": "firebase-ios-sdk", "product_memberships": [ "FirebaseAnalyticsWithoutAdIdSupport" @@ -116,7 +116,7 @@ "name": "FirebaseAnalyticsWrapper", "c99name": "FirebaseAnalyticsWrapper", "src_type": "objc", - "label": "@swiftpkg_firebase_ios_sdk//:FirebaseAnalyticsWrapper", + "label": "@swiftpkg_firebase_ios_sdk//:FirebaseAnalyticsWrapper_FirebaseAnalyticsWrapper", "package_identity": "firebase-ios-sdk", "product_memberships": [ "FirebaseAnalytics", @@ -185,6 +185,7 @@ "c99name": "FirebaseAuthCombineSwift", "src_type": "swift", "label": "@swiftpkg_firebase_ios_sdk//:FirebaseCombineSwift_Sources_Auth_FirebaseAuthCombineSwift", + "modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseCombineSwift_Sources_Auth_FirebaseAuthCombineSwift_modulemap", "package_identity": "firebase-ios-sdk", "product_memberships": [ "FirebaseAuthCombine-Community" @@ -195,6 +196,7 @@ "c99name": "FirebaseFirestoreCombineSwift", "src_type": "swift", "label": "@swiftpkg_firebase_ios_sdk//:FirebaseCombineSwift_Sources_Firestore_FirebaseFirestoreCombineSwift", + "modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseCombineSwift_Sources_Firestore_FirebaseFirestoreCombineSwift_modulemap", "package_identity": "firebase-ios-sdk", "product_memberships": [ "FirebaseFirestoreCombine-Community" @@ -205,6 +207,7 @@ "c99name": "FirebaseFunctionsCombineSwift", "src_type": "swift", "label": "@swiftpkg_firebase_ios_sdk//:FirebaseCombineSwift_Sources_Functions_FirebaseFunctionsCombineSwift", + "modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseCombineSwift_Sources_Functions_FirebaseFunctionsCombineSwift_modulemap", "package_identity": "firebase-ios-sdk", "product_memberships": [ "FirebaseFunctionsCombine-Community" @@ -215,6 +218,7 @@ "c99name": "FirebaseStorageCombineSwift", "src_type": "swift", "label": "@swiftpkg_firebase_ios_sdk//:FirebaseCombineSwift_Sources_Storage_FirebaseStorageCombineSwift", + "modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseCombineSwift_Sources_Storage_FirebaseStorageCombineSwift_modulemap", "package_identity": "firebase-ios-sdk", "product_memberships": [ "FirebaseStorageCombine-Community" @@ -240,6 +244,7 @@ "c99name": "FirebaseCoreInternal", "src_type": "swift", "label": "@swiftpkg_firebase_ios_sdk//:FirebaseCore_Internal_Sources_FirebaseCoreInternal", + "modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseCore_Internal_Sources_FirebaseCoreInternal_modulemap", "package_identity": "firebase-ios-sdk", "product_memberships": [ "FirebaseAnalytics", @@ -341,6 +346,7 @@ "c99name": "FirebaseFunctions", "src_type": "swift", "label": "@swiftpkg_firebase_ios_sdk//:FirebaseFunctions_Sources_FirebaseFunctions", + "modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseFunctions_Sources_FirebaseFunctions_modulemap", "package_identity": "firebase-ios-sdk", "product_memberships": [ "FirebaseFunctionsCombine-Community", @@ -406,6 +412,7 @@ "c99name": "FirebaseMLModelDownloader", "src_type": "swift", "label": "@swiftpkg_firebase_ios_sdk//:FirebaseMLModelDownloader_Sources_FirebaseMLModelDownloader", + "modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseMLModelDownloader_Sources_FirebaseMLModelDownloader_modulemap", "package_identity": "firebase-ios-sdk", "product_memberships": [ "FirebaseMLModelDownloader" @@ -447,6 +454,7 @@ "c99name": "FirebaseRemoteConfigSwift", "src_type": "swift", "label": "@swiftpkg_firebase_ios_sdk//:FirebaseRemoteConfigSwift_Sources_FirebaseRemoteConfigSwift", + "modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseRemoteConfigSwift_Sources_FirebaseRemoteConfigSwift_modulemap", "package_identity": "firebase-ios-sdk", "product_memberships": [ "FirebaseRemoteConfigSwift" @@ -484,6 +492,7 @@ "c99name": "FirebaseStorage", "src_type": "swift", "label": "@swiftpkg_firebase_ios_sdk//:FirebaseStorage_Sources_FirebaseStorage", + "modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseStorage_Sources_FirebaseStorage_modulemap", "package_identity": "firebase-ios-sdk", "product_memberships": [ "FirebaseStorageCombine-Community", @@ -680,7 +689,7 @@ "name": "GoogleDataTransport", "c99name": "GoogleDataTransport", "src_type": "objc", - "label": "@swiftpkg_googledatatransport//:GoogleDataTransport", + "label": "@swiftpkg_googledatatransport//:GoogleDataTransport_GoogleDataTransport", "package_identity": "googledatatransport", "product_memberships": [ "GoogleDataTransport" @@ -921,6 +930,7 @@ "c99name": "PromisesTestHelpers", "src_type": "swift", "label": "@swiftpkg_promises//:Sources_PromisesTestHelpers", + "modulemap_label": "@swiftpkg_promises//:Sources_PromisesTestHelpers_modulemap", "package_identity": "promises", "product_memberships": [ "PromisesTestHelpers" @@ -1235,7 +1245,7 @@ "name": "GoogleDataTransport", "type": "library", "target_labels": [ - "@swiftpkg_googledatatransport//:GoogleDataTransport" + "@swiftpkg_googledatatransport//:GoogleDataTransport_GoogleDataTransport" ] }, { diff --git a/gazelle/internal/reslog/rule_resolution_test.go b/gazelle/internal/reslog/rule_resolution_test.go index 0b86e69c1..904bae771 100644 --- a/gazelle/internal/reslog/rule_resolution_test.go +++ b/gazelle/internal/reslog/rule_resolution_test.go @@ -57,6 +57,7 @@ func TestRuleResolution(t *testing.T) { "Custom", swiftpkg.SwiftSourceType, newLabel("com_github_example_custom", "", "Custom"), + nil, swift.HTTPArchivePkgIdentity, nil, ), diff --git a/gazelle/internal/swift/BUILD.bazel b/gazelle/internal/swift/BUILD.bazel index abd30f542..0168e6ec6 100644 --- a/gazelle/internal/swift/BUILD.bazel +++ b/gazelle/internal/swift/BUILD.bazel @@ -12,7 +12,6 @@ go_library( "code_dir.go", "dependency_index.go", "doc.go", - "file_info.go", "files.go", "find_rules.go", "http_archive.go", @@ -63,7 +62,6 @@ go_test( "bzlmod_test.go", "code_dir_test.go", "dependency_index_test.go", - "file_info_test.go", "files_test.go", "http_archive_test.go", "is_builtin_module_test.go", diff --git a/gazelle/internal/swift/bazel_label.go b/gazelle/internal/swift/bazel_label.go index 3821ec585..db97d409b 100644 --- a/gazelle/internal/swift/bazel_label.go +++ b/gazelle/internal/swift/bazel_label.go @@ -8,6 +8,8 @@ import ( "github.com/cgrindel/rules_swift_package_manager/gazelle/internal/swiftpkg" ) +const modulemapLabelNameSuffix = "_modulemap" + // BazelLabelFromTarget creates a Bazel label from a Swift target. // The logic in this function must stay in sync with // pkginfo_targets.bazel_label_name_from_parts() in the Starlark code. @@ -24,3 +26,10 @@ func BazelLabelFromTarget(repoName string, target *swiftpkg.Target) *label.Label lbl := label.New(repoName, "", name) return &lbl } + +// ModulemapBazelLabelFromTargetLabel creates a Bazel label for a modulemap target from the +// corresponding target label. +func ModulemapBazelLabelFromTargetLabel(lbl *label.Label) *label.Label { + mml := label.New(lbl.Repo, lbl.Pkg, lbl.Name+modulemapLabelNameSuffix) + return &mml +} diff --git a/gazelle/internal/swift/bazel_label_test.go b/gazelle/internal/swift/bazel_label_test.go index e0bc726db..b33008623 100644 --- a/gazelle/internal/swift/bazel_label_test.go +++ b/gazelle/internal/swift/bazel_label_test.go @@ -43,3 +43,10 @@ func TestBazelLabelFromTarget(t *testing.T) { assert.Equal(t, &expected, actual, tt.msg) } } + +func TestModulemapBazelLabelFromTargetLabel(t *testing.T) { + targetLabel := label.New("example_cool_repo", "bzlpkg", "foo") + actual := swift.ModulemapBazelLabelFromTargetLabel(&targetLabel) + expected := label.New("example_cool_repo", "bzlpkg", "foo_modulemap") + assert.Equal(t, &expected, actual) +} diff --git a/gazelle/internal/swift/http_archive.go b/gazelle/internal/swift/http_archive.go index 459abad2c..9ac6d16f5 100644 --- a/gazelle/internal/swift/http_archive.go +++ b/gazelle/internal/swift/http_archive.go @@ -56,7 +56,7 @@ func NewHTTPArchiveFromRule(r *rule.Rule, repoRoot string) (*HTTPArchive, error) moduleName := ModuleName(br) l := label.New(repoName, "", br.Name()) m := NewModule(moduleName, moduleName, swiftpkg.SwiftSourceType, &l, - HTTPArchivePkgIdentity, nil) + nil, HTTPArchivePkgIdentity, nil) modules = append(modules, m) } diff --git a/gazelle/internal/swift/module.go b/gazelle/internal/swift/module.go index 5a60f0ac0..a305198b8 100644 --- a/gazelle/internal/swift/module.go +++ b/gazelle/internal/swift/module.go @@ -15,14 +15,17 @@ type Module struct { C99name string SrcType swiftpkg.SourceType Label *label.Label + ModulemapLabel *label.Label PkgIdentity string ProductMemberships []string } +// NewModule creates a new module. func NewModule( name, c99name string, srcType swiftpkg.SourceType, bzlLabel *label.Label, + modulemapLabel *label.Label, pkgIdentity string, pms []string, ) *Module { @@ -33,6 +36,7 @@ func NewModule( Label: bzlLabel, PkgIdentity: pkgIdentity, ProductMemberships: pms, + ModulemapLabel: modulemapLabel, } } @@ -45,13 +49,25 @@ func NewModuleFromLabelStruct( pkgIdentity string, pms []string, ) *Module { - return NewModule(name, c99name, srcType, &bzlLabel, pkgIdentity, pms) + return NewModule(name, c99name, srcType, &bzlLabel, nil, pkgIdentity, pms) } // NewModuleFromTarget returns a module from the specified Swift target. func NewModuleFromTarget(repoName, pkgIdentity string, t *swiftpkg.Target) (*Module, error) { lbl := BazelLabelFromTarget(repoName, t) - return NewModule(t.Name, t.C99name, t.SrcType, lbl, pkgIdentity, t.ProductMemberships), nil + var mml *label.Label + if t.SwiftFileInfos.RequiresModulemap() { + mml = ModulemapBazelLabelFromTargetLabel(lbl) + } + return NewModule( + t.Name, + t.C99name, + t.SrcType, + lbl, + mml, + pkgIdentity, + t.ProductMemberships, + ), nil } // LabelStr returns the label string for module. @@ -78,10 +94,12 @@ type moduleJSONData struct { C99name string `json:"c99name"` SrcType swiftpkg.SourceType `json:"src_type"` Label string `json:"label"` + ModulemapLabel string `json:"modulemap_label,omitempty"` PkgIdentity string `json:"package_identity"` ProductMemberships []string `json:"product_memberships"` } +// MarshalJSON customizes the marshalling of a module to JSON. func (m *Module) MarshalJSON() ([]byte, error) { var pms []string if len(m.ProductMemberships) > 0 { @@ -97,9 +115,13 @@ func (m *Module) MarshalJSON() ([]byte, error) { PkgIdentity: m.PkgIdentity, ProductMemberships: pms, } + if m.ModulemapLabel != nil { + jd.ModulemapLabel = m.ModulemapLabel.String() + } return json.Marshal(&jd) } +// UnmarshalJSON customizes the unmarshalling of a module from JSON. func (m *Module) UnmarshalJSON(b []byte) error { var jd moduleJSONData if err := json.Unmarshal(b, &jd); err != nil { @@ -109,7 +131,15 @@ func (m *Module) UnmarshalJSON(b []byte) error { if err != nil { return err } - newm := NewModule(jd.Name, jd.C99name, jd.SrcType, &l, jd.PkgIdentity, jd.ProductMemberships) + var mml *label.Label + if jd.ModulemapLabel != "" { + mmLabel, err := label.Parse(jd.ModulemapLabel) + if err != nil { + return err + } + mml = &mmLabel + } + newm := NewModule(jd.Name, jd.C99name, jd.SrcType, &l, mml, jd.PkgIdentity, jd.ProductMemberships) *m = *newm return nil } diff --git a/gazelle/internal/swift/module_test.go b/gazelle/internal/swift/module_test.go index 26c010063..eda93b84e 100644 --- a/gazelle/internal/swift/module_test.go +++ b/gazelle/internal/swift/module_test.go @@ -12,7 +12,7 @@ import ( func TestModule(t *testing.T) { t.Run("label string", func(t *testing.T) { l := label.New("my_repo", "path/to/pkg", "Foo") - m := swift.NewModule("Foo", "Foo", swiftpkg.SwiftSourceType, &l, "my-repo", nil) + m := swift.NewModule("Foo", "Foo", swiftpkg.SwiftSourceType, &l, nil, "my-repo", nil) actual := m.LabelStr() expected := swift.NewLabelStr(&l) assert.Equal(t, expected, actual) diff --git a/gazelle/internal/swift/rules_from_srcs.go b/gazelle/internal/swift/rules_from_srcs.go index 7de2ba933..443ab8701 100644 --- a/gazelle/internal/swift/rules_from_srcs.go +++ b/gazelle/internal/swift/rules_from_srcs.go @@ -5,6 +5,7 @@ import ( "github.com/bazelbuild/bazel-gazelle/language" "github.com/bazelbuild/bazel-gazelle/rule" + "github.com/cgrindel/rules_swift_package_manager/gazelle/internal/swiftpkg" mapset "github.com/deckarep/golang-set/v2" ) @@ -14,7 +15,7 @@ func RulesFromSrcs( srcs []string, defaultModuleName string, ) []*rule.Rule { - fileInfos := NewFileInfosFromRelPaths(args.Dir, srcs) + fileInfos := swiftpkg.NewSwiftFileInfosFromRelPaths(args.Dir, srcs) swiftImports, moduleType := collectSwiftInfo(fileInfos) shouldSetVis := shouldSetVisibility(args) @@ -31,10 +32,10 @@ func RulesFromSrcs( return rules } -var guiModules = mapset.NewSet[string]("AppKit", "UIKit", "SwiftUI") +var guiModules = mapset.NewSet("AppKit", "UIKit", "SwiftUI") // Returns the imports and the module typ -func collectSwiftInfo(fileInfos []*FileInfo) ([]string, ModuleType) { +func collectSwiftInfo(fileInfos []*swiftpkg.SwiftFileInfo) ([]string, ModuleType) { importsGUIModules := false hasTestFiles := false hasMain := false diff --git a/gazelle/internal/swiftcfg/swift_config_test.go b/gazelle/internal/swiftcfg/swift_config_test.go index 0d03f5c3e..4ada6eea8 100644 --- a/gazelle/internal/swiftcfg/swift_config_test.go +++ b/gazelle/internal/swiftcfg/swift_config_test.go @@ -59,7 +59,8 @@ func TestWriteAndReadDependencyIndex(t *testing.T) { origsc.DependencyIndexPath = filepath.Join(dir, swiftcfg.DefaultDependencyIndexBasename) lbl := label.New("cool_repo", "Sources/Foo", "Foo") - m := swift.NewModule("Foo", "Foo", swiftpkg.SwiftSourceType, &lbl, "", []string{"ProductA"}) + m := swift.NewModule("Foo", "Foo", swiftpkg.SwiftSourceType, &lbl, nil, "", + []string{"ProductA"}) origsc.DependencyIndex.AddModule(m) // Write the index diff --git a/gazelle/internal/swiftpkg/BUILD.bazel b/gazelle/internal/swiftpkg/BUILD.bazel index 65c5ea97f..a672e03f8 100644 --- a/gazelle/internal/swiftpkg/BUILD.bazel +++ b/gazelle/internal/swiftpkg/BUILD.bazel @@ -12,6 +12,7 @@ go_library( "platform.go", "product.go", "source_type.go", + "swift_file_info.go", "target.go", "target_dependency.go", ], @@ -33,6 +34,7 @@ go_test( "module_type_test.go", "package_info_test.go", "source_type_test.go", + "swift_file_info_test.go", ], # The PackageInfo tests use SwiftBin to create an actual Swift package. To use Swift, the test # needs to be executed outside of the sandbox. diff --git a/gazelle/internal/swiftpkg/package_info.go b/gazelle/internal/swiftpkg/package_info.go index 2fd892722..729276f1e 100644 --- a/gazelle/internal/swiftpkg/package_info.go +++ b/gazelle/internal/swiftpkg/package_info.go @@ -66,7 +66,7 @@ func NewPackageInfo(sw swiftbin.Executor, dir string) (*PackageInfo, error) { // Ignore phantom targets (i.e., appear in description but not in the dump) continue } - t, err := NewTargetFromManifestInfo(&descT, dumpT, prodNames) + t, err := NewTargetFromManifestInfo(dir, &descT, dumpT, prodNames) if err != nil { return nil, fmt.Errorf("failed to create target for %s: %w", descT.Name, err) } diff --git a/gazelle/internal/swift/file_info.go b/gazelle/internal/swiftpkg/swift_file_info.go similarity index 53% rename from gazelle/internal/swift/file_info.go rename to gazelle/internal/swiftpkg/swift_file_info.go index 1c166b2e8..5201a08ea 100644 --- a/gazelle/internal/swift/file_info.go +++ b/gazelle/internal/swiftpkg/swift_file_info.go @@ -1,4 +1,4 @@ -package swift +package swiftpkg import ( "bufio" @@ -11,18 +11,19 @@ import ( "strings" ) -// FileInfo represents source file information that is pertinent for Swift build file generation. -type FileInfo struct { - Rel string - Abs string - Imports []string - IsTest bool - ContainsMain bool +// SwiftFileInfo represents source file information that is pertinent for Swift build file generation. +type SwiftFileInfo struct { + Rel string + Abs string + Imports []string + IsTest bool + ContainsMain bool + HasObjcDirective bool } -// NewFileInfoFromReader returns file info for a source file. -func NewFileInfoFromReader(rel, abs string, reader io.Reader) *FileInfo { - fi := FileInfo{ +// NewSwiftFileInfoFromReader returns file info for a source file. +func NewSwiftFileInfoFromReader(rel, abs string, reader io.Reader) *SwiftFileInfo { + fi := SwiftFileInfo{ Rel: rel, Abs: abs, IsTest: testSuffixes.HasSuffix(rel) || TestDirSuffixes.IsUnderDirWithSuffix(rel), @@ -45,6 +46,8 @@ func NewFileInfoFromReader(rel, abs string, reader io.Reader) *FileInfo { fi.ContainsMain = true case match[mainFnReSubexpIdx] != nil: fi.ContainsMain = true + case match[objcDirReSubexpIdx] != nil: + fi.HasObjcDirective = true } } } @@ -53,28 +56,28 @@ func NewFileInfoFromReader(rel, abs string, reader io.Reader) *FileInfo { return &fi } -// NewFileInfoFromSrc returns file info for source file's contents as a string. -func NewFileInfoFromSrc(rel, abs, src string) *FileInfo { - return NewFileInfoFromReader(rel, abs, strings.NewReader(src)) +// NewSwiftFileInfoFromSrc returns file info for source file's contents as a string. +func NewSwiftFileInfoFromSrc(rel, abs, src string) *SwiftFileInfo { + return NewSwiftFileInfoFromReader(rel, abs, strings.NewReader(src)) } -// NewFileInfoFromPath returns file info from a filesystem path. -func NewFileInfoFromPath(rel, abs string) (*FileInfo, error) { +// NewSwiftFileInfoFromPath returns file info from a filesystem path. +func NewSwiftFileInfoFromPath(rel, abs string) (*SwiftFileInfo, error) { file, err := os.Open(abs) if err != nil { return nil, err } - return NewFileInfoFromReader(rel, abs, file), nil + return NewSwiftFileInfoFromReader(rel, abs, file), nil } -// NewFileInfosFromRelPaths returns a slice of file information for the source files in a directory. -func NewFileInfosFromRelPaths(dir string, srcs []string) []*FileInfo { - fileInfos := make([]*FileInfo, len(srcs)) +// NewSwiftFileInfosFromRelPaths returns a slice of file information for the source files in a directory. +func NewSwiftFileInfosFromRelPaths(dir string, srcs []string) SwiftFileInfos { + fileInfos := make(SwiftFileInfos, len(srcs)) for idx, src := range srcs { abs := filepath.Join(dir, src) - fi, err := NewFileInfoFromPath(src, abs) + fi, err := NewSwiftFileInfoFromPath(src, abs) if err != nil { - log.Printf("failed to create swift.FileInfo for %s. %s", abs, err) + log.Printf("failed to create swift.SwiftFileInfo for %s. %s", abs, err) continue } fileInfos[idx] = fi @@ -90,7 +93,11 @@ func buildSwiftRegexp() *regexp.Regexp { importStmt := `\bimport\s*(?P` + ident + `)\b` mainAnnotation := `^\s*(?P@main\b)` mainFnDecl := `(?P\bstatic\s+func\s+main\b)` - swiftReSrc := strings.Join([]string{commentLine, importStmt, mainAnnotation, mainFnDecl}, "|") + objcDirective := `(?P@objc\b)` + swiftReSrc := strings.Join( + []string{commentLine, importStmt, mainAnnotation, mainFnDecl, objcDirective}, + "|", + ) return regexp.MustCompile(swiftReSrc) } @@ -100,6 +107,7 @@ const ( importReSubexpIdx = 2 mainAnnotationReSubexpIdx = 3 mainFnReSubexpIdx = 4 + objcDirReSubexpIdx = 5 ) type fileSuffixes []string @@ -115,8 +123,10 @@ func (fs fileSuffixes) HasSuffix(path string) bool { var testSuffixes = fileSuffixes{"Tests.swift", "Test.swift"} +// DirSuffixes provides a means for testing a path having one of the listed suffixes. type DirSuffixes []string +// HasSuffix checks if the path has one of the suffixes. func (ds DirSuffixes) HasSuffix(path string) bool { for _, suffix := range ds { if strings.HasSuffix(path, suffix) { @@ -126,6 +136,7 @@ func (ds DirSuffixes) HasSuffix(path string) bool { return false } +// IsUnderDirWithSuffix checks if the path has a directory that includes one of the suffixes. func (ds DirSuffixes) IsUnderDirWithSuffix(path string) bool { if path == "." || path == "" || path == "/" { return false @@ -137,4 +148,18 @@ func (ds DirSuffixes) IsUnderDirWithSuffix(path string) bool { return ds.IsUnderDirWithSuffix(dir) } +// TestDirSuffixes lists the suffixes used for Swift test directories. var TestDirSuffixes = DirSuffixes{"Tests", "Test"} + +// SwiftFileInfos represents a collection of SwiftFileInfo instances. +type SwiftFileInfos []*SwiftFileInfo + +// RequiresModulemap determines whether a modulemap target will be generated for this target. +func (sfis SwiftFileInfos) RequiresModulemap() bool { + for _, sfi := range sfis { + if sfi.HasObjcDirective { + return true + } + } + return false +} diff --git a/gazelle/internal/swift/file_info_test.go b/gazelle/internal/swiftpkg/swift_file_info_test.go similarity index 53% rename from gazelle/internal/swift/file_info_test.go rename to gazelle/internal/swiftpkg/swift_file_info_test.go index 101b8f238..1af28e337 100644 --- a/gazelle/internal/swift/file_info_test.go +++ b/gazelle/internal/swiftpkg/swift_file_info_test.go @@ -1,21 +1,21 @@ -package swift_test +package swiftpkg_test import ( "path/filepath" "testing" - "github.com/cgrindel/rules_swift_package_manager/gazelle/internal/swift" + "github.com/cgrindel/rules_swift_package_manager/gazelle/internal/swiftpkg" "github.com/stretchr/testify/assert" ) const fileInfoRel = "Foo/Hello.swift" const fileInfoAbs = "/path/to/workspace/Foo/Hello.swift" -func TestNewFileInfoFromSrc(t *testing.T) { +func TestNewSwiftFileInfoFromSrc(t *testing.T) { t.Run("main function without imports", func(t *testing.T) { - actual := swift.NewFileInfoFromSrc( + actual := swiftpkg.NewSwiftFileInfoFromSrc( fileInfoRel, fileInfoAbs, mainFnWithoutImports) - expected := &swift.FileInfo{ + expected := &swiftpkg.SwiftFileInfo{ Rel: fileInfoRel, Abs: fileInfoAbs, ContainsMain: true, @@ -23,9 +23,9 @@ func TestNewFileInfoFromSrc(t *testing.T) { assert.Equal(t, expected, actual) }) t.Run("main annotation with imports", func(t *testing.T) { - actual := swift.NewFileInfoFromSrc( + actual := swiftpkg.NewSwiftFileInfoFromSrc( fileInfoRel, fileInfoAbs, mainAnnotationWithImports) - expected := &swift.FileInfo{ + expected := &swiftpkg.SwiftFileInfo{ Rel: fileInfoRel, Abs: fileInfoAbs, Imports: []string{"ArgumentParser", "Foundation"}, @@ -36,8 +36,8 @@ func TestNewFileInfoFromSrc(t *testing.T) { t.Run("test file", func(t *testing.T) { rel := "FooTests/HelloTests.swift" abs := filepath.Join("Tests", rel) - actual := swift.NewFileInfoFromSrc(rel, abs, testFile) - expected := &swift.FileInfo{ + actual := swiftpkg.NewSwiftFileInfoFromSrc(rel, abs, testFile) + expected := &swiftpkg.SwiftFileInfo{ Rel: rel, Abs: abs, Imports: []string{"DateUtils", "XCTest"}, @@ -49,8 +49,8 @@ func TestNewFileInfoFromSrc(t *testing.T) { t.Run("main under test directory", func(t *testing.T) { rel := "FooTests/main.swift" abs := filepath.Join("Tests", rel) - actual := swift.NewFileInfoFromSrc(rel, abs, mainForTest) - expected := &swift.FileInfo{ + actual := swiftpkg.NewSwiftFileInfoFromSrc(rel, abs, mainForTest) + expected := &swiftpkg.SwiftFileInfo{ Rel: rel, Abs: abs, Imports: []string{"XCTest"}, @@ -59,22 +59,65 @@ func TestNewFileInfoFromSrc(t *testing.T) { } assert.Equal(t, expected, actual) }) + t.Run("test file", func(t *testing.T) { + rel := "Foo/Hello.swift" + abs := filepath.Join("Sources", rel) + actual := swiftpkg.NewSwiftFileInfoFromSrc(rel, abs, objcDirective) + expected := &swiftpkg.SwiftFileInfo{ + Rel: rel, + Abs: abs, + Imports: []string{"Foundation"}, + HasObjcDirective: true, + } + assert.Equal(t, expected, actual) + }) } func TestDirSuffixes(t *testing.T) { - actual := swift.TestDirSuffixes.IsUnderDirWithSuffix("Sources/Foo/Bar.swift") + actual := swiftpkg.TestDirSuffixes.IsUnderDirWithSuffix("Sources/Foo/Bar.swift") assert.False(t, actual) - actual = swift.TestDirSuffixes.IsUnderDirWithSuffix("Tests/FooTests/Bar.swift") + actual = swiftpkg.TestDirSuffixes.IsUnderDirWithSuffix("Tests/FooTests/Bar.swift") assert.True(t, actual) - actual = swift.TestDirSuffixes.IsUnderDirWithSuffix("Tests/FooTests/Chicken/Bar.swift") + actual = swiftpkg.TestDirSuffixes.IsUnderDirWithSuffix("Tests/FooTests/Chicken/Bar.swift") assert.True(t, actual) - actual = swift.TestDirSuffixes.IsUnderDirWithSuffix("Tests/Bar.swift") + actual = swiftpkg.TestDirSuffixes.IsUnderDirWithSuffix("Tests/Bar.swift") assert.True(t, actual) } +func TestSwiftFileInfos(t *testing.T) { + tests := []struct { + msg string + fileInfos swiftpkg.SwiftFileInfos + exp bool + }{ + { + msg: "no files have objc directive", + fileInfos: swiftpkg.SwiftFileInfos{ + {HasObjcDirective: false}, + {HasObjcDirective: false}, + {HasObjcDirective: false}, + }, + exp: false, + }, + { + msg: "a file has an objc directive", + fileInfos: swiftpkg.SwiftFileInfos{ + {HasObjcDirective: false}, + {HasObjcDirective: true}, + {HasObjcDirective: false}, + }, + exp: true, + }, + } + for _, tt := range tests { + actual := tt.fileInfos.RequiresModulemap() + assert.Equal(t, tt.exp, actual, tt.msg) + } +} + const mainFnWithoutImports = ` @main public struct Hello { @@ -87,7 +130,7 @@ public struct Hello { ` const mainAnnotationWithImports = ` -// Intentionally not sorted to ensure that FileInfo imports is sorted +// Intentionally not sorted to ensure that SwiftFileInfo imports is sorted import Foundation import ArgumentParser @@ -130,3 +173,14 @@ XCTMain([ ]) #endif ` + +const objcDirective = ` +#if canImport(Combine) && swift(>=5.0) + + import Foundation + + // Make this class discoverable from Objective-C. Don't instantiate directly. + @objc(FIRCombineFirestoreLibrary) private class __CombineFirestoreLibrary: NSObject {} + +#endif +` diff --git a/gazelle/internal/swiftpkg/target.go b/gazelle/internal/swiftpkg/target.go index 9ace93e0e..6dcfd6e1f 100644 --- a/gazelle/internal/swiftpkg/target.go +++ b/gazelle/internal/swiftpkg/target.go @@ -2,6 +2,7 @@ package swiftpkg import ( "fmt" + "path/filepath" "github.com/cgrindel/rules_swift_package_manager/gazelle/internal/spdesc" "github.com/cgrindel/rules_swift_package_manager/gazelle/internal/spdump" @@ -60,10 +61,13 @@ type Target struct { CSettings *ClangSettings SrcType SourceType ProductMemberships []string + // SwiftFileInfos will only be populated if the target is a Swift target. + SwiftFileInfos SwiftFileInfos } // NewTargetFromManifestInfo returns a Swift target from manifest information. func NewTargetFromManifestInfo( + pkgPath string, descT *spdesc.Target, dumpT *spdump.Target, prodNames mapset.Set[string], @@ -84,6 +88,14 @@ func NewTargetFromManifestInfo( return nil, fmt.Errorf( "unrecognized spdump.TargetType %v for %s target", dumpT.Type, dumpT.Name) } + moduleType := NewModuleType(descT.ModuleType) + srcType := NewSourceType(moduleType, descT.Sources) + + var swiftFileInfos SwiftFileInfos + if srcType == SwiftSourceType { + targetPath := filepath.Join(pkgPath, descT.Path) + swiftFileInfos = NewSwiftFileInfosFromRelPaths(targetPath, descT.Sources) + } // GH046: A Swift plugin can have a dependency on an executable target. In this case, we // want to add the target as a data dependency. @@ -102,8 +114,6 @@ func NewTargetFromManifestInfo( return nil, err } - moduleType := NewModuleType(descT.ModuleType) - // The description JSON can contain phantom products. These are products that are not declared // in the original package manifest (e.g., executable target not associated with a product). We // do not want to include these phantoms as SPM package resolution may not include all of the @@ -124,8 +134,9 @@ func NewTargetFromManifestInfo( Sources: descT.Sources, Dependencies: tdeps, CSettings: cSettings, - SrcType: NewSourceType(moduleType, descT.Sources), + SrcType: srcType, ProductMemberships: prodMemberships, + SwiftFileInfos: swiftFileInfos, }, nil } diff --git a/swiftpkg/internal/deps_indexes.bzl b/swiftpkg/internal/deps_indexes.bzl index 141f74f57..2a93603e1 100644 --- a/swiftpkg/internal/deps_indexes.bzl +++ b/swiftpkg/internal/deps_indexes.bzl @@ -76,16 +76,29 @@ def _new(modules = [], products = [], packages = []): ) def _new_module_from_dict(mod_dict): + modulemap_label = None + modulemap_label_str = mod_dict.get("modulemap_label", "") + if modulemap_label_str != "": + modulemap_label = bazel_labels.parse(modulemap_label_str) + return _new_module( name = mod_dict["name"], c99name = mod_dict["c99name"], src_type = mod_dict.get("src_type", "unknown"), label = bazel_labels.parse(mod_dict["label"]), + modulemap_label = modulemap_label, package_identity = mod_dict["package_identity"], product_memberships = mod_dict["product_memberships"], ) -def _new_module(name, c99name, src_type, label, package_identity, product_memberships): +def _new_module( + name, + c99name, + src_type, + label, + package_identity, + product_memberships, + modulemap_label = None): validations.in_list( src_types.all_values, src_type, @@ -96,6 +109,7 @@ def _new_module(name, c99name, src_type, label, package_identity, product_member c99name = c99name, src_type = src_type, label = label, + modulemap_label = modulemap_label, package_identity = package_identity, product_memberships = product_memberships, ) @@ -211,10 +225,12 @@ def _labels_for_module(module, depender_src_type): # See `swiftpkg_build_files.bzl` for more information. labels.append(_modulemap_label_for_module(module)) - elif depender_src_type == src_types.objc and module.src_type == src_types.swift: + elif (depender_src_type == src_types.objc and + module.src_type == src_types.swift and + module.modulemap_label != None): # If an Objc module wants to @import a Swift module, it will need the # modulemap target. - labels.append(_modulemap_label_for_module(module)) + labels.append(module.modulemap_label) return labels diff --git a/swiftpkg/tests/deps_indexes_tests.bzl b/swiftpkg/tests/deps_indexes_tests.bzl index 92bebd375..671bd94a8 100644 --- a/swiftpkg/tests/deps_indexes_tests.bzl +++ b/swiftpkg/tests/deps_indexes_tests.bzl @@ -2,7 +2,7 @@ load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest") load("@cgrindel_bazel_starlib//bzllib:defs.bzl", "bazel_labels") -load("//swiftpkg/internal:deps_indexes.bzl", "deps_indexes") +load("//swiftpkg/internal:deps_indexes.bzl", "deps_indexes", "src_types") def _new_from_json_test(ctx): env = unittest.begin(ctx) @@ -178,12 +178,21 @@ def _labels_for_module_test(ctx): ], ), struct( - msg = "Objc library depends upon Swift library", + msg = "Objc library depends upon Swift library without modulemap", dep_module = "@example_cool_repo//:Foo", depender_module = "@example_cool_repo//:ObjcLibrary", exp = [ bazel_labels.parse("@example_cool_repo//:Foo"), - bazel_labels.parse("@example_cool_repo//:Foo_modulemap"), + # bazel_labels.parse("@example_cool_repo//:Foo_modulemap"), + ], + ), + struct( + msg = "Objc library depends upon Swift library with modulemap", + dep_module = "@example_another_repo//Sources/Foo", + depender_module = "@example_cool_repo//:ObjcLibrary", + exp = [ + bazel_labels.parse("@example_another_repo//Sources/Foo"), + bazel_labels.parse("@example_another_repo//Sources/Foo:Foo_modulemap"), ], ), ] @@ -304,6 +313,48 @@ def _resolve_product_with_ctx_test(ctx): resolve_product_with_ctx_test = unittest.make(_resolve_product_with_ctx_test) +def _new_module_test(ctx): + env = unittest.begin(ctx) + + tests = [ + struct( + msg = "no modulemap_label", + modulemap_label = None, + ), + struct( + msg = "with modulemap_label", + modulemap_label = bazel_labels.parse( + "@example_cool_repo//:Foo_modulemap", + ), + ), + ] + for t in tests: + actual = deps_indexes.new_module( + name = "Foo", + c99name = "Foo", + src_type = src_types.swift, + label = bazel_labels.parse("@example_cool_repo//:Foo"), + package_identity = "example_cool_repo", + product_memberships = ["Bar"], + modulemap_label = t.modulemap_label, + ) + asserts.equals(env, "Foo", actual.name, t.msg) + asserts.equals(env, "Foo", actual.c99name, t.msg) + asserts.equals(env, src_types.swift, actual.src_type, t.msg) + asserts.equals( + env, + bazel_labels.parse("@example_cool_repo//:Foo"), + actual.label, + t.msg, + ) + asserts.equals(env, "example_cool_repo", actual.package_identity, t.msg) + asserts.equals(env, ["Bar"], actual.product_memberships, t.msg) + asserts.equals(env, t.modulemap_label, actual.modulemap_label, t.msg) + + return unittest.end(env) + +new_module_test = unittest.make(_new_module_test) + def deps_indexes_test_suite(): return unittest.suite( "deps_indexes_tests", @@ -314,6 +365,7 @@ def deps_indexes_test_suite(): resolve_product_test, resolve_product_with_ctx_test, labels_for_module_test, + new_module_test, ) _deps_index_json = """ @@ -340,6 +392,7 @@ _deps_index_json = """ "c99name": "Foo", "src_type": "swift", "label": "@example_another_repo//Sources/Foo", + "modulemap_label": "@example_another_repo//Sources/Foo:Foo_modulemap", "package_identity": "example-another-repo", "product_memberships": ["Foo"] },