RedundantSuspendModifier - False positive

[Skyleiger]

Expected Behavior

Method should be allowed to be a suspension function, because it calls another suspension function.

Observed Behavior

Detekt throws a violation: Function has redundant suspend modifier. [RedundantSuspendModifier]

Context

This is the method:

    private suspend fun loadWeatherData(
        location: Location,
        interval: ZonedDateTimeInterval,
    ): Map<ZonedDateTime, WeatherData>? {
        require(interval.isBounded()) { "Provided interval must be bounded: $interval" }

        return try {
            webClient
                .get()
                .uri { uriBuilder ->
                    uriBuilder
                        .path("/weather")
                        .queryParam("lat", location.latitude)
                        .queryParam("lon", location.longitude)
                        .queryParam("date", interval.start.toOffsetDateTime().toString())
                        .queryParam("last_date", interval.endExclusive.toOffsetDateTime().toString())
                        .queryParam("tz", "UTC")
                        .queryParam("units", "dwd")
                        .build()
                }
                .retrieve()
                .awaitBody<String>()
                .let { BrightSkyResponseParser.parseWeatherData(it) }
        } catch (exception: WebClientResponseException) {
            logger.error(exception) { "Failed to load weather data from BrightSky" }
            null
        }
    }

It used the spring reactive web client with kotlin extensions. The method "awaitBody" is an inline suspension function.

Your Environment

Detekt version 1.23.8, using the detekt-maven-plugin.

         <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-dependency-plugin</artifactId>
                <version>3.2.0</version>
                <executions>
                    <execution>
                        <id>generate-classpath-var</id>
                        <phase>package</phase>
                        <goals>
                            <goal>build-classpath</goal>
                        </goals>
                        <configuration>
                            <outputProperty>generated.classpath</outputProperty>
                            <silent>true</silent>
                        </configuration>
                    </execution>
                </executions>
            </plugin>
            <plugin>
                <groupId>com.github.ozsie</groupId>
                <artifactId>detekt-maven-plugin</artifactId>
                <version>${detekt-maven-plugin.version}</version>
                <configuration>
                    <config>detekt.yml</config>
                    <classPath>${generated.classpath}</classPath>
                    <jvmTarget>${kotlin.compiler.jvmTarget}</jvmTarget>
                    <!-- Exclude script files, because they throw an exception -->
                    <excludes>**/resources/**/*.kts</excludes>
                </configuration>
                <executions>
                    <execution>
                        <phase>verify</phase>
                        <goals>
                            <goal>ctr</goal>
                        </goals>
                    </execution>
                </executions>
            </plugin>

[detekt-ci]

This issue is stale because it has been open 90 days with no activity. Please comment or this will be closed in 7 days.

[atulgpt]

Hi @Skyleiger can you create a more simpler case which gives the false positive?

[Skyleiger]

Hi @Skyleiger can you create a more simpler case which gives the false positive?

Unfortunately, I can no longer reproduce the problem with the latest version. I will open the issue again if I notice the error again. Thank you for your help.

[realdadfish]

@Skyleiger I just saw the same issue, however just on CI (which is even weirder), for this very simply piece of code in a multiplatform module:

commonMain:

@Suppress("UseDataClass")
expect class OAuthClientSettings(dispatcherProvider: DispatcherProvider) {
    val environment: Environment
    internal val authFlowFactory: CodeAuthFlowFactory
    internal val store: SettingsStore
}

androidMain:

@Suppress("ktlint:standard:backing-property-naming")
actual class OAuthClientSettings actual constructor(private val dispatcherProvider: DispatcherProvider) {
    private var _environment: Environment = Environment.DEV
    actual val environment: Environment
        get() = _environment

    private var _authFlowFactory = WeakReference<AndroidCodeAuthFlowFactory>(null)
    internal actual val authFlowFactory: CodeAuthFlowFactory
        get() = _authFlowFactory.get() ?: error("authFlowFactory was garbage collected")

    private var _store: SettingsStore? = null
    internal actual val store: SettingsStore
        get() = _store ?: error("register has not been called")

    // This is a false positive that comes up from Detekt, as `withContext` is a suspending method
    // we naturally need to make the caller of that method suspending, too.
    @Suppress("RedundantSuspendModifier")
    suspend fun configure(environment: Environment, context: Context) {
        _environment = environment
        withContext(dispatcherProvider.ioDispatcher) {
            _store = AndroidEncryptedPreferencesSettingsStore(context)
        }
    }
}

For now we're suppressing it, as we aren't even sure why this pops up right now, as this happens in an unrelated part of the code just in a PR (our develop does not bring this up at all) where an external library was upgraded (and we held back a few transitive production dependencies this provided). This should, by no means, trigger build-time linting issues like this.