Skip to content

Commit

Permalink
ignore suppress/noinspection names which don't fit the FindingName pa…
Browse files Browse the repository at this point in the history
…ttern

fixes #827
  • Loading branch information
RBusarow committed Feb 5, 2023
1 parent 714b679 commit c8db7d4
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2021-2022 Rick Busarow
* Copyright (C) 2021-2023 Rick Busarow
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
Expand Down Expand Up @@ -52,6 +52,18 @@ data class FindingName(

companion object {

/**
* @return a [FindingName] if [maybeFindingName] is `kebab-case`, otherwise `null`.
* @since 0.12.4
*/
fun safe(maybeFindingName: String): FindingName? {
return if (CaseMatcher.KebabCaseMatcher().matches(maybeFindingName)) {
FindingName(maybeFindingName)
} else {
null
}
}

@Deprecated("This will be removed soon.")
fun migrateLegacyIdOrNull(legacyID: String, logger: McLogger): String? {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2021-2022 Rick Busarow
* Copyright (C) 2021-2023 Rick Busarow
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
Expand Down Expand Up @@ -33,7 +33,6 @@ import modulecheck.parsing.gradle.model.ProjectPath.StringProjectPath
import modulecheck.reporting.logging.McLogger
import modulecheck.utils.lazy.ResetManager
import modulecheck.utils.lazy.lazyResets
import modulecheck.utils.mapToSet
import modulecheck.utils.remove

abstract class AbstractDependenciesBlock(
Expand All @@ -53,7 +52,9 @@ abstract class AbstractDependenciesBlock(
allModuleDeclarations.forEach { (configuredModule, declarations) ->

val cached = getOrPut(configuredModule) {
blockSuppressed.mapTo(mutableSetOf()) { FindingName(it) }
blockSuppressed
.mapNotNull { FindingName.safe(it) }
.mapTo(mutableSetOf()) { it }
}

declarations.forEach { moduleDependencyDeclaration ->
Expand Down Expand Up @@ -170,7 +171,7 @@ abstract class AbstractDependenciesBlock(
}

private fun Collection<String>.asFindingNames(): Set<FindingName> {
return mapToSet { FindingName(it) }
return mapNotNull { FindingName.safe(it) }.toSet()
}

override fun getOrEmpty(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2021-2022 Rick Busarow
* Copyright (C) 2021-2023 Rick Busarow
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
Expand All @@ -21,7 +21,6 @@ import modulecheck.parsing.gradle.dsl.PluginsBlock
import modulecheck.reporting.logging.McLogger
import modulecheck.utils.lazy.ResetManager
import modulecheck.utils.lazy.lazyResets
import modulecheck.utils.mapToSet
import java.io.File

abstract class AbstractPluginsBlock(
Expand All @@ -48,7 +47,7 @@ abstract class AbstractPluginsBlock(
_allDeclarations.forEach { pluginDeclaration ->

val cached = getOrPut(pluginDeclaration) {
blockSuppressed.mapTo(mutableSetOf()) { FindingName(it) }
blockSuppressed.mapNotNullTo(mutableSetOf()) { FindingName.safe(it) }
}

cached += pluginDeclaration.suppressed.updateOldSuppresses()
Expand Down Expand Up @@ -101,7 +100,7 @@ abstract class AbstractPluginsBlock(
}

private fun Collection<String>.asFindingNames(): Set<FindingName> {
return mapToSet { FindingName(it) }
return mapNotNullTo(mutableSetOf()) { FindingName.safe(it) }
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2021-2022 Rick Busarow
* Copyright (C) 2021-2023 Rick Busarow
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
Expand Down Expand Up @@ -80,6 +80,20 @@ internal class GroovyDependenciesBlockParserTest : BaseTest() {
)
}

@Test
fun `suppression which doesn't match finding name regex should be ignored`() = parse(
"""
//noinspection DSL_SCOPE_VIOLATION
dependencies {
api project(':core:android')
api project(':core:jvm')
}
"""
) {

allSuppressions.values.flatten() shouldBe emptyList()
}

@Test
fun `declaration with noinspection with old IDs should include suppressed with argument`() =
parse(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2021-2022 Rick Busarow
* Copyright (C) 2021-2023 Rick Busarow
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
Expand Down Expand Up @@ -119,6 +119,19 @@ internal class GroovyPluginsBlockParserTest : BaseTest() {
)
}

@Test
fun `suppression which doesn't match finding name regex should be ignored`() = parse(
"""
//noinspection DSL_SCOPE_VIOLATION
plugins {
id 'com.squareup.anvil'
}
"""
) {

allSuppressions.values.flatten() shouldBe emptyList()
}

@Test
fun `suppress with old ID should be updated`() = parse(
"""
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2021-2022 Rick Busarow
* Copyright (C) 2021-2023 Rick Busarow
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
Expand Down Expand Up @@ -111,6 +111,24 @@ internal class KotlinDependenciesBlockParserTest : ProjectTest() {
)
}

@Test
fun `suppression which doesn't match finding name regex should be ignored`() {
val block = parser
.parse(
"""
@Suppress("DSL_SCOPE_VIOLATION")
dependencies {
api(project(":core:android"))
api(project(":core:jvm"))
@Suppress("DSL_SCOPE_VIOLATION")
testImplementation(project(":core:test"))
}
"""
).single()

block.allSuppressions.values.flatten() shouldBe emptyList()
}

@Test
fun `declaration with annotation should include annotation with argument`() {
val block = parser
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2021-2022 Rick Busarow
* Copyright (C) 2021-2023 Rick Busarow
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
Expand Down Expand Up @@ -104,6 +104,20 @@ internal class KotlinPluginsBlockParserTest : BaseTest() {
)
}

@Test
fun `suppression which doesn't match finding name regex should be ignored`() {
val block = parse(
"""
@Suppress("DSL_SCOPE_VIOLATION")
plugins {
id("com.squareup.anvil")
}
"""
)

block.allSuppressions.values.flatten() shouldBe emptyList()
}

@Test
fun `suppressed id function`() {
val block = parse(
Expand Down

0 comments on commit c8db7d4

Please sign in to comment.