Fix target linter warnings for target products that allow dots and hyphens #6290
Conversation
There was a problem hiding this comment.
Thanks for fixing this @kapitoshka438. Only a small thing regarding the message and we should be good to go.
| } | ||
|
|
||
| if target.productName.unicodeScalars.allSatisfy(allowed.contains) == false { | ||
| let reason = | ||
| "Invalid product name '\(target.productName)'. This string must contain only alphanumeric (A-Z,a-z,0-9)\(allowsDot ? ", period (.)" : ""), and underscore (_) characters." | ||
| "Invalid product name '\(target.productName)'. This string must contain only alphanumeric (A-Z,a-z,0-9)\(allowsExtraCharacters ? ", period (.), hyphen (-)" : ""), and underscore (_) characters." |
There was a problem hiding this comment.
This message is not true for products that are not included in productsAllowingExtraCharacters, is it?
There was a problem hiding this comment.
Period and hyphen are allowed in product name for app, bundle, commandLineTool and, I believe, many other types of targets but framework.
I can check all types where Xcode automatically replaces these characters with underscore.
There was a problem hiding this comment.
Well, I've checked almost all targets in Xcode and there's only one type where it transforms the product name by replacing dots and hyphens with underscores. It's Framework. When you add it, Xcode automatically sets its Product Name to $(TARGET_NAME:c99extidentifier) so that, for example, a target named my.frame-work gets Product Name as my_frame_work.
I've also tried to add such targets (.staticFramework and .framework) in a Tuist project and it works differently, replacing only hyphens: My.Frame-work -> My.Frame_work.
There was a problem hiding this comment.
I think we need to make changes here:
if let productName {
self.productName = productName
} else if self.product == .framework || self.product == .staticFramework {
self.productName = "$(TARGET_NAME:c99extidentifier)"
} else {
self.productName = "$(TARGET_NAME)"
}
There was a problem hiding this comment.
Thanks @kapitoshka438
I wonder if a different bug is causing the product names for Braze to get mangled and not propagated correctly - I don't have a lot of context on the SPM integration but from what I can see in the package manifest all target names seem sane with no hyphens or dots!
https://github.com/braze-inc/braze-swift-sdk/blob/main/Package.swift
| if let productName { | ||
| self.productName = productName | ||
| } else if product == .framework || product == .staticFramework { | ||
| self.productName = "$(TARGET_NAME:c99extidentifier)" | ||
| } else { | ||
| self.productName = "$(TARGET_NAME)" | ||
| } |
There was a problem hiding this comment.
productName is used in a few different contexts within Tuist in addition to setting the underlying build setting. While Xcode can recognise variables - sadly Tuist can't and will regress a few things within it where the productName is relied upon to derive various xcodeProj element / product names.
The logic needs to continue to be defined in tuist code as opposed to via variables.
There was a problem hiding this comment.
Ah makes sense. I'll revert this. It's not really related to the original issue. Anyway it's a pity that we can't use such variables. But maybe someday we will. )
| self.productName = productName ?? name.sanitizedModuleName | ||
| if let productName { | ||
| self.productName = productName | ||
| } else if product == .framework || product == .staticFramework { |
There was a problem hiding this comment.
is the discrepancy of what constitutes of valid product names documented anywhere? I wasn't aware frameworks, libraries an applications have different rules 🤔
There was a problem hiding this comment.
It's the result of my manual research. I created an empty project in Xcode and created all possible types of targets with names containing dots and hyphens. Framework is the only target where Xcode transforms Product Name by setting it to "$(TARGET_NAME:c99extidentifier)". Product Name for all other targets was just "$(TARGET_NAME)"
There was a problem hiding this comment.
I would suggest to try to bring the same approach in Tuist someday. But, taking into account your previous comment, not in this PR.
This is a bug of the product name linter. Look at the package name, not targets names. The package name contains hyphens (braze-swift-sdk). And there are bundle products which names look like "{packageName}_{name}.bundle". Bundles product names can contain hyphens but the linter allows only dots and only for apps and commandLineTools. |
kapitoshka438
force-pushed
the
target-linter-allow-hyphen
branch
2 times, most recently
from
a5b3f3c
to
77cc617
Compare
kapitoshka438
force-pushed
the
target-linter-allow-hyphen
branch
from
77cc617
to
f92c62b
Compare
pepicrft
added
the
changelog:fixed
Resolves #6289
Short description 📝
This PR fixes the target linter to allow dot (".") and hyphen ("-") in some types of target products.
How to test the changes locally 🧐
Generate the
app_with_spm_dependenciesfixture. Target linter warnings for BrazeUI dependency bundles should not appear anymore.Contributor checklist ✅
mise run lint:fixReviewer checklist ✅
changelog:added,changelog:fixed, orchangelog:changed, and the title is usable as a changelog entry