Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to suppress Lint warnings #57

Closed
juandiana opened this issue Feb 14, 2023 · 10 comments
Closed

Unable to suppress Lint warnings #57

juandiana opened this issue Feb 14, 2023 · 10 comments

Comments

@juandiana
Copy link

I'm not being able to suppress ComposeModifierMissing lint warnings

@SuppressLint("ComposeModifierMissing")
@Composable
fun Screen(
    onAbortButtonClicked: () -> Unit,
    onContinueButtonClicked: () -> Unit
)

Both, the lint task (gradlew lint) fails and Android Studio displays it with red underlines.

Android Studio version: Electric Eel 2022.1.1 Patch 1
AGP version: 7.4.1
Compose lint version: 1.0.0

Let me know if you need any more information to reproduce

@ZacSweers
Copy link
Collaborator

I think this may be a bug in lint itself since compose modifies the function signature, you'll need to raise this on the android issue tracker as we don't control suppressibility. We can re-open if they say otherwise.

In the meantime, a possible workaround may be to use a baseline file instead.

@ZacSweers ZacSweers closed this as not planned Won't fix, can't repro, duplicate, stale Feb 14, 2023
@jzbrooks
Copy link
Contributor

jzbrooks commented Jan 3, 2024

We ran into this also. I reported the issue and pointed to this issue for reference.

@ZacSweers
Copy link
Collaborator

Another possible workaround is that lint does support kotlin's @Suppress annotation too, so you could try that

@jzbrooks
Copy link
Contributor

jzbrooks commented Jan 3, 2024

No dice

@tnorbye
Copy link

tnorbye commented Jan 10, 2024

Hmmmm, I went to investigate that bug (https://issuetracker.google.com/issues/318532624) and I can't reproduce it.

I opened the Slack lint project, opened ModifierMissingDetectorTest, went to the first test with violations, and added @Suppress("ComposeModifierMissing") in front of the first @composable annotation -- and when I run the test it shows that it's now suppressed the warning. I also used @android.annotation.SuppressLint("ComposeModifierMissing") and that worked too.

(By the way, in that codebase I see this --
// This mode is irrelevant to our test and totally untestable with stringy outputs
override val skipTestModes: Array =
arrayOf(TestMode.PARENTHESIZED, TestMode.SUPPRESSIBLE, TestMode.TYPE_ALIAS)
I removed TestMode.SUPPRESSIBLE and it seems to work fine here. I don't really understand the comment about "totally untestable with stringy outputs".)

@jzbrooks
Copy link
Contributor

Thanks for looking into it. How likely is that related to the test harness or detector implementation? I skimmed the implementation and nothing stuck out to me as obviously wrong.

Here's a repro: https://github.com/jzbrooks/SuppressLintWoe

Screenshot 2024-01-10 at 9 01 25 AM

@tnorbye
Copy link

tnorbye commented Jan 10, 2024

Thanks for the reproduce case! I debugged this -- and discovered that this was fixed back in August. Basically, this lint check passes in the Kotlin PSI element as the context location (used for suppress directive lookup) instead of the UAST element, and that wasn't handled right (until August). If you update to 8.3.0-beta01 or later it should work.

@jzbrooks
Copy link
Contributor

Great. Thanks!

@juandiana
Copy link
Author

Thanks for the update @tnorbye & the additional comment no-inspection fix, and @jzbrooks for bringing this up again and providing a repro to get it fixed!

@ZacSweers
Copy link
Collaborator

Awesome! Thanks all for the discussion here.

(By the way, in that codebase I see this --
// This mode is irrelevant to our test and totally untestable with stringy outputs
override val skipTestModes: Array =
arrayOf(TestMode.PARENTHESIZED, TestMode.SUPPRESSIBLE, TestMode.TYPE_ALIAS)
I removed TestMode.SUPPRESSIBLE and it seems to work fine here. I don't really understand the comment about "totally untestable with stringy outputs".)

I believe this was copied over from some internal lints, and predate knowledge of lint tests' ability to specify a specific test mode when asserting error/warning strings :). I'm slowly removing all these in #209

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants