Fix RestrictToDetector implementation of LIBRARY_GROUP_PREFIX to work for longer groupIds [187523902]

Assigned

Feature Request

Status Update

No update yet.

Description

al...@google.com

created issue #1

May 7, 2021 06:16PM

Currently, the implementation of sameLibraryGroupPrefix warns when androidx.paging uses a LIBRARY_GROUP_PREFIX-scoped API from androidx.arch.core. This seems to be an oversight, given the TODO in the method and the original request for LIBRARY_GROUP_PREFIX being specifically targeted to AndroidX usage.

        fun sameLibraryGroupPrefix(group1: String, group2: String): Boolean {
            // TODO: Allow group1.startsWith(group2) || group2.startsWith(group1) ?

What we really want, I think, is just firstIndexOf rather than lastIndexOf:

            val i1 = group1.lastIndexOf('.')
            val i2 = group2.lastIndexOf('.')
            if (i2 != i1 || i1 == -1) {
                return false
            }

Comments

al...@google.com <al...@google.com> #2Jul 15, 2021 03:43PM

Apologies Jeremy, I realized we have a tracking ticket for the sheet APIs specifically: b/261409034 Feel free to keep this issue or mark as duplicate of the other one.

al...@google.com <al...@google.com> #3Jul 15, 2021 03:43PM

Sounds good. I will make it a dup.

al...@google.com <al...@google.com> #4Aug 2, 2021 01:54PM

This is a Lint issue.

al...@google.com <al...@google.com> #5Aug 2, 2021 01:54PM

Moving back to root component again for triage.

cm...@google.com <cm...@google.com> Aug 3, 2021 03:26PM

Assigned to an...@google.com.

tn...@google.com <tn...@google.com> #6Aug 3, 2021 05:57PM

Reassigned to tn...@google.com.

I'd be happy to just apply that patch (shown in #1) if that's what you want; I think AndroidX is the main or only client of the prefix type (it was added to support AndroidX) -- just checking that that's exactly what you want. Is there a particular test case you want to pass and one you want to fail?

al...@google.com <al...@google.com> #7Aug 3, 2021 06:00PM

Is there a particular test case you want to pass and one you want to fail?

The androidx.arch.core from androidx.paging scenario captures it well. This should pass.

Something like accessing androidx.arch.core from com.google.android.material should fail.

au...@google.com <au...@google.com> #8Aug 3, 2021 06:10PM

How far up should it go?

Should com.google.android.material be able to access com.android.tools?

al...@google.com <al...@google.com> #9Aug 3, 2021 06:48PM

Ooh, yeah, good point. I think the Material folks would expect something like com.google.android.material or com.google.android as the scope. Which indicates @RestrictTo(prefix = "androidx") would be useful. This would also be useful for ErrorProne's implementation since it's not aware of Maven group IDs.

al...@google.com <al...@google.com> #10Aug 3, 2021 06:50PM

Though at that point the restriction is not LIBRARY_GROUP_PREFIX, it's more of a relaxed package visibility. Equivalent for androidx, since we require the Java package to match the Maven group ID, but not generally true.

al...@google.com <al...@google.com> #11Aug 2, 2023 07:25PM

Tor, I'd be happy to implement this if the approach in comment#3 (@RestrictTo(prefix = "androidx")) seems reasonable. If so, feel free to reassign to me.