refactor(cocoapods): Move Podspec to a dedicated file #8880
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8880 +/- ##
=========================================
Coverage 67.54% 67.54%
Complexity 1166 1166
=========================================
Files 244 244
Lines 7775 7775
Branches 865 865
=========================================
Hits 5252 5252
Misses 2167 2167
Partials 356 356
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Separate the CocoaPods model from the analyzer. Signed-off-by: Frank Viernau <[email protected]>
fviernau
force-pushed
the
cocoapods-extract-podspec
branch
2 times, most recently
from
d2a38fc to
d4a1741
Compare
| @@ -211,9 +210,9 @@ class CocoaPods( | |||
| return null | |||
| } | |||
|
|
|||
| val podspecFile = File(podspecCommand.stdout.trim()) | |||
| val podspec = File(podspecCommand.stdout.trim()).readText().parsePodspec() | |||
There was a problem hiding this comment.
Nit: I'd have a slight tendency towards keeping podspecFile as a separate variable here to not do too many conversions in this single line, and eventually ease inspecting variables when debugging.
| private object LicenseSerializer : JsonTransformingSerializer<String>(String.serializer()) { | ||
| override fun transformDeserialize(element: JsonElement): JsonElement = | ||
| if (element is JsonObject) { | ||
| val type: String = (element["type"] as JsonPrimitive).content |
There was a problem hiding this comment.
The explicit type should be dropped, and the jsonPrimitive extension function should be used (which requires explicit handling of the null case for element["type"]).
There was a problem hiding this comment.
Can you please provide a rationale for the latter? (I believe it is fine to assume that type is not null here, which this code does.)
There was a problem hiding this comment.
Explicit handling of the null case for element["type"] is a side effect of using jsonPrimitive, not the primary goal. But I think it's a good side effect as the original code also handled the null case explicitly via textValueOrEmpty(). If you want to document that you expect the field to exist in any case (and throw an exception otherwise), then you should use getValue() instead of [].
There was a problem hiding this comment.
Note: I kept the explicit type for readability.
Prepare for migrating the parsing to KxS. Signed-off-by: Frank Viernau <[email protected]>
fviernau
force-pushed
the
cocoapods-extract-podspec
branch
from
59c65fa to
8096d70
Compare
When the `source` node contains boolean properties with unquoted values,
such as [1], the deserialization of the source property to a
`Map<String, String>` with KxS fails by default. So, introduce a
dedicated data class to prepare for migrating to KxS and to improve the
readability.
[1]: "source": {
"git": "https://github.com/AFNetworking/AFNetworking.git",
"tag": "3.2.1",
"submodules": true
}
Signed-off-by: Frank Viernau <[email protected]>
Signed-off-by: Frank Viernau <[email protected]>
fviernau
force-pushed
the
cocoapods-extract-podspec
branch
from
8096d70 to
a615014
Compare
| import kotlinx.serialization.json.JsonObject | ||
| import kotlinx.serialization.json.JsonPrimitive | ||
| import kotlinx.serialization.json.JsonTransformingSerializer | ||
| import kotlinx.serialization.json.jsonPrimitive |
There was a problem hiding this comment.
This import is present twice now.
There was a problem hiding this comment.
Not sure why Detekt did not report this, though.
There was a problem hiding this comment.
You've overlooked that the first character differs, e.g. j vs. J.
| @@ -37,6 +40,9 @@ dependencies { | |||
| implementation(projects.utils.ortUtils) | |||
| implementation(projects.utils.spdxUtils) | |||
|
|
|||
| implementation(libs.kotlinx.serialization.core) | |||
| implementation(libs.kotlinx.serialization.json) | |||
|
|
|||
| implementation(libs.jackson.annotations) | |||
There was a problem hiding this comment.
The Jackson dependencies should be removed.
There was a problem hiding this comment.
Not yet. The lockfile parsing also needs to be ported.
Separate the CocoaPods model from the analyzer.