Skip to content

Commit

Permalink
chore: ktlint & swiftlint version bumps (#904)
Browse files Browse the repository at this point in the history
* Update swiftlint and ktlint versions; address new lint warnings; disable questionable property naming rule in ktlint.

* Instead of diabling a rule, add suppress to relevant exceptions as needed.

* Make warnings fail swiftlint as well.

* Remove extra dash

---------

Co-authored-by: Sichan Yoo <chanyoo@amazon.com>
  • Loading branch information
sichanyoo and Sichan Yoo authored Feb 6, 2025
1 parent 165edc8 commit e3deec9
Show file tree
Hide file tree
Showing 238 changed files with 5,589 additions and 3,617 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ jobs:
if: github.repository == 'smithy-lang/smithy-swift' || github.event_name == 'pull_request'
runs-on: ubuntu-latest
container:
image: ghcr.io/realm/swiftlint:0.54.0
image: ghcr.io/realm/swiftlint:0.58.2
steps:
- name: Checkout sources
uses: actions/checkout@v2
- name: Run swiftlint
run: swiftlint --reporter github-actions-logging
run: swiftlint --strict --reporter github-actions-logging
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import struct Foundation.TimeInterval
import struct Foundation.Date
import func Foundation.pow

actor ClientSideRateLimiter: Sendable {
actor ClientSideRateLimiter {

// these are constants defined in Retry Behavior 2.0
let minFillRate: Double = 0.5
Expand Down
5 changes: 2 additions & 3 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@ val ktlint by configurations.creating
val ktlintVersion: String by project

dependencies {
ktlint("com.pinterest:ktlint:$ktlintVersion")
ktlint("com.pinterest.ktlint:ktlint-cli:$ktlintVersion")
}

val lintPaths = listOf(
"smithy-swift-codegen/src/**/*.kt",
"smithy-swift-codegen-test-utils/src/**/*.kt"
"smithy-swift-codegen/**/*.kt",
)

tasks.register<JavaExec>("ktlint") {
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ kotlinxBenchmarkVersion=0.3.1

# FIXME - junit5 not working
junitVersion=5.6.2
ktlintVersion=0.40.0
ktlintVersion=1.5.0
kotestVersion=4.6.0
jacocoVersion=0.8.7
1 change: 0 additions & 1 deletion settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,3 @@ rootProject.name = "smithy-swift"

include("smithy-swift-codegen")
include("smithy-swift-codegen-test")
include("smithy-swift-codegen-test-utils")
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ class AuthSchemeResolverGenerator {
private fun renderServiceSpecificAuthResolverParamsStruct(
serviceIndex: ServiceIndex,
ctx: ProtocolGenerator.GenerationContext,
writer: SwiftWriter
writer: SwiftWriter,
) {
writer.apply {
openBlock(
"public struct ${getSdkId(ctx)}${SmithyHTTPAuthAPITypes.AuthSchemeResolverParams.name}: \$N {",
"}",
SmithyHTTPAuthAPITypes.AuthSchemeResolverParams
SmithyHTTPAuthAPITypes.AuthSchemeResolverParams,
) {
write("public let operation: \$N", SwiftTypes.String)

Expand All @@ -71,7 +71,10 @@ class AuthSchemeResolverGenerator {
}
}

private fun renderEndpointParamFields(ctx: ProtocolGenerator.GenerationContext, writer: SwiftWriter) {
private fun renderEndpointParamFields(
ctx: ProtocolGenerator.GenerationContext,
writer: SwiftWriter,
) {
writer.apply {
val ruleSetNode = ctx.service.getTrait<EndpointRuleSetTrait>()?.ruleSet
val ruleSet = if (ruleSetNode != null) EndpointRuleSet.fromNode(ruleSetNode) else null
Expand All @@ -87,12 +90,15 @@ class AuthSchemeResolverGenerator {
}
}

private fun renderServiceSpecificAuthResolverProtocol(ctx: ProtocolGenerator.GenerationContext, writer: SwiftWriter) {
private fun renderServiceSpecificAuthResolverProtocol(
ctx: ProtocolGenerator.GenerationContext,
writer: SwiftWriter,
) {
writer.apply {
openBlock(
"public protocol ${getServiceSpecificAuthSchemeResolverName(ctx)}: \$N {",
"}",
SmithyHTTPAuthAPITypes.AuthSchemeResolver
SmithyHTTPAuthAPITypes.AuthSchemeResolver,
) {
// This is just a parent protocol that all auth scheme resolvers of a given service must conform to.
write("// Intentionally empty.")
Expand All @@ -105,16 +111,19 @@ class AuthSchemeResolverGenerator {
private fun renderServiceSpecificDefaultResolver(
serviceIndex: ServiceIndex,
ctx: ProtocolGenerator.GenerationContext,
writer: SwiftWriter
writer: SwiftWriter,
) {
val sdkId = getSdkId(ctx)

// Model-based auth scheme resolver is internal implementation detail for services that use rules based resolvers,
// and is used as fallback only if endpoint resolver returns no valid auth scheme(s).
val usesRulesBasedResolver = usesRulesBasedAuthResolver(ctx)
val defaultResolverName =
if (usesRulesBasedResolver) "InternalModeled$sdkId$AUTH_SCHEME_RESOLVER"
else "Default$sdkId$AUTH_SCHEME_RESOLVER"
if (usesRulesBasedResolver) {
"InternalModeled$sdkId$AUTH_SCHEME_RESOLVER"
} else {
"Default$sdkId$AUTH_SCHEME_RESOLVER"
}

// Model-based auth scheme resolver should be private internal impl detail if service uses rules-based resolver.
val accessModifier = if (usesRulesBasedResolver) "private" else "public"
Expand All @@ -126,15 +135,15 @@ class AuthSchemeResolverGenerator {
"}",
accessModifier,
defaultResolverName,
serviceSpecificAuthResolverProtocol
serviceSpecificAuthResolverProtocol,
) {
write("")
renderResolveAuthSchemeMethod(serviceIndex, ctx, writer)
write("")
renderConstructParametersMethod(
serviceIndex.getEffectiveAuthSchemes(ctx.service).contains(SigV4Trait.ID),
ctx,
writer
writer,
)
}
}
Expand All @@ -143,7 +152,7 @@ class AuthSchemeResolverGenerator {
private fun renderResolveAuthSchemeMethod(
serviceIndex: ServiceIndex,
ctx: ProtocolGenerator.GenerationContext,
writer: SwiftWriter
writer: SwiftWriter,
) {
val sdkId = getSdkId(ctx)
val serviceParamsName = sdkId + SmithyHTTPAuthAPITypes.AuthSchemeResolverParams.name
Expand All @@ -159,7 +168,10 @@ class AuthSchemeResolverGenerator {
write("var validAuthOptions = [\$N]()", SmithyHTTPAuthAPITypes.AuthOption)
// Cast params to service specific params object
openBlock("guard let serviceParams = params as? \$L else {", "}", serviceParamsName) {
write("throw \$N.authError(\"Service specific auth scheme parameters type must be passed to auth scheme resolver.\")", SmithyTypes.ClientError)
write(
"throw \$N.authError(\"Service specific auth scheme parameters type must be passed to auth scheme resolver.\")",
SmithyTypes.ClientError,
)
}
// Render switch block
renderSwitchBlock(serviceIndex, ctx, writer)
Expand All @@ -172,7 +184,7 @@ class AuthSchemeResolverGenerator {
private fun renderSwitchBlock(
serviceIndex: ServiceIndex,
ctx: ProtocolGenerator.GenerationContext,
writer: SwiftWriter
writer: SwiftWriter,
) {
writer.apply {
// Switch block for iterating over operation name cases
Expand All @@ -187,11 +199,12 @@ class AuthSchemeResolverGenerator {
opShape.hasTrait(UnsignedPayloadTrait::class.java)
) {
val opName = it.name.toLowerCamelCase()
val validSchemesForOp = serviceIndex.getEffectiveAuthSchemes(
ctx.service,
it,
ServiceIndex.AuthSchemeMode.NO_AUTH_AWARE
)
val validSchemesForOp =
serviceIndex.getEffectiveAuthSchemes(
ctx.service,
it,
ServiceIndex.AuthSchemeMode.NO_AUTH_AWARE,
)
write("case \"$opName\":")
renderSwitchCase(validSchemesForOp, writer)
}
Expand All @@ -205,7 +218,10 @@ class AuthSchemeResolverGenerator {
}
}

private fun renderSwitchCase(schemes: Map<ShapeId, Trait>, writer: SwiftWriter) {
private fun renderSwitchCase(
schemes: Map<ShapeId, Trait>,
writer: SwiftWriter,
) {
writer.apply {
indent()
schemes.forEach {
Expand All @@ -223,10 +239,16 @@ class AuthSchemeResolverGenerator {
}
}

private fun renderSigV4AuthOption(scheme: Map.Entry<ShapeId, Trait>, writer: SwiftWriter) {
private fun renderSigV4AuthOption(
scheme: Map.Entry<ShapeId, Trait>,
writer: SwiftWriter,
) {
writer.apply {
write("var sigV4Option = \$N(schemeID: \$S)", SmithyHTTPAuthAPITypes.AuthOption, scheme.key)
write("sigV4Option.signingProperties.set(key: \$N.signingName, value: \"${(scheme.value as SigV4Trait).name}\")", SmithyHTTPAuthAPITypes.SigningPropertyKeys)
write(
"sigV4Option.signingProperties.set(key: \$N.signingName, value: \"${(scheme.value as SigV4Trait).name}\")",
SmithyHTTPAuthAPITypes.SigningPropertyKeys,
)
openBlock("guard let region = serviceParams.region else {", "}") {
write("throw \$N.authError(\"Missing region in auth scheme parameters for SigV4 auth scheme.\")", SmithyTypes.ClientError)
}
Expand All @@ -238,20 +260,23 @@ class AuthSchemeResolverGenerator {
private fun renderConstructParametersMethod(
hasSigV4: Boolean,
ctx: ProtocolGenerator.GenerationContext,
writer: SwiftWriter
writer: SwiftWriter,
) {
writer.apply {
openBlock(
"public func constructParameters(context: \$N) throws -> \$N {",
"}",
SmithyTypes.Context,
SmithyHTTPAuthAPITypes.AuthSchemeResolverParams
SmithyHTTPAuthAPITypes.AuthSchemeResolverParams,
) {
if (usesRulesBasedAuthResolver(ctx)) {
write("return try Default${getSdkId(ctx) + AUTH_SCHEME_RESOLVER}().constructParameters(context: context)")
} else {
openBlock("guard let opName = context.getOperation() else {", "}") {
write("throw \$N.dataNotFound(\"Operation name not configured in middleware context for auth scheme resolver params construction.\")", SmithyTypes.ClientError)
write(
"throw \$N.dataNotFound(\"Operation name not configured in middleware context for auth scheme resolver params construction.\")",
SmithyTypes.ClientError,
)
}
val paramType = getSdkId(ctx) + SmithyHTTPAuthAPITypes.AuthSchemeResolverParams.name
if (hasSigV4) {
Expand All @@ -269,31 +294,35 @@ class AuthSchemeResolverGenerator {
private val AUTH_SCHEME_RESOLVER = "AuthSchemeResolver"

// Utility function for checking if a service relies on endpoint resolver for auth scheme resolution
fun usesRulesBasedAuthResolver(ctx: ProtocolGenerator.GenerationContext): Boolean {
return listOf("s3", "eventbridge", "cloudfront keyvaluestore", "sesv2").contains(ctx.settings.sdkId.lowercase(Locale.US))
}
fun usesRulesBasedAuthResolver(ctx: ProtocolGenerator.GenerationContext): Boolean =
listOf("s3", "eventbridge", "cloudfront keyvaluestore", "sesv2").contains(ctx.settings.sdkId.lowercase(Locale.US))

// Utility function for returning sdkId from generation context
fun getSdkId(ctx: ProtocolGenerator.GenerationContext): String {
return if (ctx.service.hasTrait(ServiceTrait::class.java))
ctx.service.getTrait(ServiceTrait::class.java).get().sdkId.clientName()
else ctx.settings.sdkId.clientName()
}
fun getSdkId(ctx: ProtocolGenerator.GenerationContext): String =
if (ctx.service.hasTrait(ServiceTrait::class.java)) {
ctx.service
.getTrait(ServiceTrait::class.java)
.get()
.sdkId
.clientName()
} else {
ctx.settings.sdkId.clientName()
}

fun getServiceSpecificAuthSchemeResolverName(ctx: ProtocolGenerator.GenerationContext): Symbol {
return buildSymbol {
fun getServiceSpecificAuthSchemeResolverName(ctx: ProtocolGenerator.GenerationContext): Symbol =
buildSymbol {
name = "${getSdkId(ctx)}$AUTH_SCHEME_RESOLVER"
}
}
}
}

fun Parameter.toSymbol(): Symbol {
val swiftType = when (type) {
ParameterType.STRING -> SwiftTypes.String
ParameterType.BOOLEAN -> SwiftTypes.Bool
ParameterType.STRING_ARRAY -> SwiftTypes.StringArray
}
val swiftType =
when (type) {
ParameterType.STRING -> SwiftTypes.String
ParameterType.BOOLEAN -> SwiftTypes.Bool
ParameterType.STRING_ARRAY -> SwiftTypes.StringArray
}
var builder = Symbol.builder().name(swiftType.fullName)
if (!isRequired) {
builder = builder.boxed()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import software.amazon.smithy.swift.codegen.integration.ProtocolGenerator
import software.amazon.smithy.swift.codegen.integration.SwiftIntegration

class DefaultClientConfigurationIntegration : SwiftIntegration {
override fun clientConfigurations(ctx: ProtocolGenerator.GenerationContext): List<ClientConfiguration> {
return listOf(DefaultClientConfiguration(), DefaultHttpClientConfiguration())
}
override fun clientConfigurations(ctx: ProtocolGenerator.GenerationContext): List<ClientConfiguration> =
listOf(DefaultClientConfiguration(), DefaultHttpClientConfiguration())
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,14 @@ import software.amazon.smithy.swift.codegen.integration.SwiftIntegration
import software.amazon.smithy.swift.codegen.model.hasTrait
import java.util.logging.Logger

class DirectedSwiftCodegen(val context: PluginContext) :
DirectedCodegen<GenerationContext, SwiftSettings, SwiftIntegration> {
class DirectedSwiftCodegen(
val context: PluginContext,
) : DirectedCodegen<GenerationContext, SwiftSettings, SwiftIntegration> {
@Suppress("ktlint:standard:property-naming")
private val LOGGER = Logger.getLogger(javaClass.name)

override fun createSymbolProvider(directive: CreateSymbolProviderDirective<SwiftSettings>): SymbolProvider {
return SwiftSymbolProvider(directive.model(), directive.settings())
}
override fun createSymbolProvider(directive: CreateSymbolProviderDirective<SwiftSettings>): SymbolProvider =
SwiftSymbolProvider(directive.model(), directive.settings())

override fun createContext(directive: CreateContextDirective<SwiftSettings, SwiftIntegration>): GenerationContext {
val model = directive.model()
Expand All @@ -56,7 +57,7 @@ class DirectedSwiftCodegen(val context: PluginContext) :
directive.settings(),
directive.fileManifest(),
protocolGenerator,
directive.integrations()
directive.integrations(),
)
}

Expand Down Expand Up @@ -172,14 +173,16 @@ class DirectedSwiftCodegen(val context: PluginContext) :
val settings = directive.settings()
val symbolProvider = directive.symbolProvider()
val writers = context.writerDelegator()
writers.useShapeWriter(shape) { writer: SwiftWriter -> IntEnumGenerator(model, symbolProvider, writer, shape.asIntEnumShape().get(), settings).render() }
writers.useShapeWriter(
shape,
) { writer: SwiftWriter -> IntEnumGenerator(model, symbolProvider, writer, shape.asIntEnumShape().get(), settings).render() }
}

private fun resolveProtocolGenerator(
integrations: List<SwiftIntegration>,
model: Model,
service: ServiceShape,
settings: SwiftSettings
settings: SwiftSettings,
): ProtocolGenerator? {
val generators = integrations.flatMap { it.protocolGenerators }.associateBy { it.protocol }
val serviceIndex = ServiceIndex.of(model)
Expand Down
Loading

0 comments on commit e3deec9

Please sign in to comment.