@Language("sql") syntax highlighting doesn't work in Studio, shows red squiggle [377733398]

Fixed

Bug

Status Update

No update yet.

Description

py...@gmail.com

created issue #1

Nov 7, 2024 12:52AM

Component used: androidx.benchmark.macro Version used: Version 1.4.0-alpha04

Calling PerfettoTraceProcessor.Session#query with SELECT slice.name, ... shows the following error in the IDE: id, name or spec expected, got 'SELECT s...'

Comments

cc...@google.com <cc...@google.com> #2Nov 7, 2024 07:12PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

Message last modified on Nov 7, 2024 07:14PM

an...@google.com <an...@google.com> #3Nov 11, 2024 10:00PM

Reassigned to an...@google.com.

I'm not sure this is the same thing as the YouTrack issue linked above. I can repro this trivially in Android Studio:

fun query(@Language("sql") query: String) { TODO() }

fun callQuery() {
    query("SELECT * FROM foo")
}

In the above snippet, there will be a syntax error. The same does not happen in IDEA; that's usually a good clue that it's something we're doing differently in AS code. I'll take a look.

an...@google.com <an...@google.com> #4Nov 15, 2024 06:46PM

Reassigned to an...@google.com.

This issue is not specifically related to SQL. The language "sql" is not supported in Android Studio, and the same bug will happen for any unrecognized language:

fun query(@Language("asdcasdcasdcads") query: String) { TODO() }

fun callQuery() {
    query("SELECT * FROM foo")
}

The above shows the string as a syntax error, when in reality we should just be displaying it as a normal string without any syntax highlighting. (Note that the language we do support can be annotated with "RoomSql".)

This is happening because of DeviceSpecInjector.kt, which is using DeviceSpecLanguage as a default when the language is unknown. I'm unfamiliar with what that's meant to do, but I've validated that if I remove that default and replace it with a return false (indicating that injection was not done), then everything works as expected.

DeviceSpecLanguage seems to be intended for use only with the @Preview parameter, if I'm reading it correctly; but I may have misinterpreted the intent of the class. Either way, I'm redirecting this bug to the team that owns it, so that they can scope the logic more appropriately.

di...@google.com <di...@google.com> Nov 18, 2024 09:43AM

Reassigned to di...@google.com.

le...@google.com <le...@google.com> Nov 18, 2024 02:21PM

Assigned to le...@google.com.

cc...@google.com <cc...@google.com> #5Nov 18, 2024 06:23PM

This appears to have regressed in Iguana.

We added the @Language("sql") API functionality in macrobench in this change: https://android-review.git.corp.google.com/c/platform/frameworks/support/+/2447462

So as of March 6 last year, it was working as I developed that change. I tried a few versions of Studio to narrow this regression down:

Electric eel (not working, presumably not supported yet)
Flamingo (works in basic way, see 1st screenshot) - I expect AndroidX would have been on a Flamingo canary when I added the annotation
Giraffe (works)
Hedgehog (works better, see 2nd screenshot which now highlights the table name as a standard identifier)
Iguana (broken in current way, see 3rd screenshot)

I tested by creating a trivial empty project, and adding the following to the example unit test:

    fun sql(@Language("sql") param: String) {}
    @Test
    fun foo() {
        sql("select * from whatever")
    }

Paste containing exact versions I tested on (these are just the initial stable versions of each release): https://paste.googleplex.com/4747774884315136

Mon Nov 18 2024 10:12:16 GMT-0800 (Pacific Standard Time).png

16 KB

View

Download

Mon Nov 18 2024 10:16:45 GMT-0800 (Pacific Standard Time).png

16 KB

View

Download

Mon Nov 18 2024 10:21:32 GMT-0800 (Pacific Standard Time).png

16 KB

View

Download

le...@google.com <le...@google.com> Nov 19, 2024 02:14PM

Accepted by le...@google.com.

di...@google.com <di...@google.com> Feb 28, 2025 09:24AM

Assigned to le...@google.com.

am...@google.com <am...@google.com> #6Mar 4, 2025 02:33PM

Marked as fixed, reassigned to am...@google.com.

The above shows the string as a syntax error, when in reality we should just be displaying it as a normal string without any syntax highlighting

That has been fixed. DeviceSpecInjector should only be applied for @Preview annotations. The scenario from comment #5 is displayed as a normal string (no syntax error) now: http://screen/3aNg9znY8dG3v4Y.png

cc...@google.com <cc...@google.com> #7Mar 4, 2025 05:36PM

Status: Assigned (reopened)

That's better, but if it's just displayed as a string, it's still a regression from Flamingo, see #comment5

Can we get that @Language("sql") support added back to Studio?

am...@google.com <am...@google.com> #8Mar 4, 2025 05:50PM

Can we get that @Language("sql") support added back to Studio?

Would using @Language("RoomSql") work for your use case? Otherwise I'll file a feature request to the core editor team to add support for @Language("sql") and we can follow up there. I'd leave this bug focused on the issue originally reported (syntax error), which was caused by our custom device spec.

am...@google.com <am...@google.com> #9Mar 4, 2025 06:00PM

This is how the snippet from https://r.android.com/c/platform/frameworks/support/+/2447462/7/benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/perfetto/AudioUnderrunQuery.kt#29 looks like when using @Language("RoomSql") (see attachment)

Tue Mar 04 2025 17:59:22 GMT+0000 (Greenwich Mean Time).png

63 KB

View

Download

am...@google.com <am...@google.com> #10Mar 4, 2025 07:42PM

Marked as fixed.

Marking this as fixed per comment #8. Filed issue 400745325 to follow up about restoring support of @Language("sql") if this is still needed.

cc...@google.com <cc...@google.com> #11Mar 4, 2025 10:19PM

Would using @Language("RoomSql") work for your use case

Unfortunately no, "RoomSql" doesn't support includes or function declarations in my testing (see screenshots, where this worked with @Language("sql") in Giraffe, but has red squigglies in Meerkat FD Canary 7). Samples of the syntax our library's clients will call here.

Tue Mar 04 2025 14:15:06 GMT-0800 (Pacific Standard Time).png

25 KB

View

Download

Tue Mar 04 2025 14:17:00 GMT-0800 (Pacific Standard Time).png

21 KB

View

Download

Issue 377733398

Description

Issue summary

Comments

cc...@google.com <cc...@google.com> #2Nov 7, 2024 07:12PM

an...@google.com <an...@google.com> #3Nov 11, 2024 10:00PM

an...@google.com <an...@google.com> #4Nov 15, 2024 06:46PM

di...@google.com <di...@google.com> Nov 18, 2024 09:43AM

le...@google.com <le...@google.com> Nov 18, 2024 02:21PM

cc...@google.com <cc...@google.com> #5Nov 18, 2024 06:23PM

le...@google.com <le...@google.com> Nov 19, 2024 02:14PM

di...@google.com <di...@google.com> Feb 28, 2025 09:24AM

am...@google.com <am...@google.com> #6Mar 4, 2025 02:33PM

cc...@google.com <cc...@google.com> #7Mar 4, 2025 05:36PM

am...@google.com <am...@google.com> #8Mar 4, 2025 05:50PM

am...@google.com <am...@google.com> #9Mar 4, 2025 06:00PM

am...@google.com <am...@google.com> #10Mar 4, 2025 07:42PM

cc...@google.com <cc...@google.com> #11Mar 4, 2025 10:19PM

Add comment

Issue metadata