feat: Implement string literal conversion code actions #1931
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WillLillis
force-pushed
the
str_code_action
branch
2 times, most recently
from
12437b9
to
6cf5235
Compare
WillLillis
force-pushed
the
str_code_action
branch
4 times, most recently
from
3ff66ed
to
bdd29e0
Compare
Techatrix
requested changes
Jul 18, 2024
There was a problem hiding this comment.
I have not yet taken a close look at the implementation but here are some cases that are incorrect:
const s1 = "\t";
const s2 = "\r";
const s3 =
\\"
;
const s4 =
\\\
;I would suggest to take a look at the std.zig and std.zig.string_literal namespaces. There are some very useful functions there.
tests/lsp_features/code_actions.zig
Outdated
| @@ -415,3 +502,51 @@ fn testAutofixOptions(before: []const u8, after: []const u8, want_zir: bool) !vo | |||
|
|
|||
| try std.testing.expectEqualStrings(after, handle.tree.source); | |||
| } | |||
|
|
|||
| fn testUserCodeAction(source: []const u8, expected: []const u8) !void { | |||
There was a problem hiding this comment.
Testing by applying all code actions at the given cursor position works great if there is only choice at a given location. This makes the function not usable when there are multiple different code actions at the same cursor position. Code actions should ideally be tested by providing the cursor position and code action name.
This is not something that needs to be changed for this PR but you still free to do this change for future improvements.
There was a problem hiding this comment.
Makes sense. I took a pass at adding the expected action kind to the test function.
I was able to fix the tab issue pretty readily, but ran into issues with the carriage return case. After some digging, I found ziglang/zig-spec#38, which explicitly says that both carriage returns and tabs aren't allowed in multiline strings. However, a quick grep over the zig codebase says otherwise for tabs at least Output[lillis@DESKTOP-1H3O3N6] ~/projects/zig (master)
❯ grep -rnP --include \*.zig '\\\\.*[\t]+.*$' . ⏎
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:306: \\ <key>ProductBuildVersion</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:307: \\ <string>7W98</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:308: \\ <key>ProductCopyright</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:309: \\ <string>Apple Computer, Inc. 1983-2004</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:310: \\ <key>ProductName</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:311: \\ <string>Mac OS X</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:312: \\ <key>ProductUserVisibleVersion</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:313: \\ <string>10.3.9</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:314: \\ <key>ProductVersion</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:315: \\ <string>10.3.9</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:326: \\ <key>ProductBuildVersion</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:327: \\ <string>19G68</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:328: \\ <key>ProductCopyright</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:329: \\ <string>1983-2020 Apple Inc.</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:330: \\ <key>ProductName</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:331: \\ <string>Mac OS X</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:332: \\ <key>ProductUserVisibleVersion</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:333: \\ <string>10.15.6</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:334: \\ <key>ProductVersion</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:335: \\ <string>10.15.6</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:336: \\ <key>iOSSupportVersion</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:337: \\ <string>13.6</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:348: \\ <key>ProductBuildVersion</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:349: \\ <string>20A2408</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:350: \\ <key>ProductCopyright</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:351: \\ <string>1983-2020 Apple Inc.</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:352: \\ <key>ProductName</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:353: \\ <string>macOS</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:354: \\ <key>ProductUserVisibleVersion</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:355: \\ <string>11.0</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:356: \\ <key>ProductVersion</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:357: \\ <string>11.0</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:358: \\ <key>iOSSupportVersion</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:359: \\ <string>14.2</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:370: \\ <key>ProductBuildVersion</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:371: \\ <string>20C63</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:372: \\ <key>ProductCopyright</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:373: \\ <string>1983-2020 Apple Inc.</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:374: \\ <key>ProductName</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:375: \\ <string>macOS</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:376: \\ <key>ProductUserVisibleVersion</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:377: \\ <string>11.1</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:378: \\ <key>ProductVersion</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:379: \\ <string>11.1</string>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:380: \\ <key>iOSSupportVersion</key>
./zig-out/lib/zig/std/zig/system/darwin/macos.zig:381: \\ <string>14.3</string>
./zig-out/lib/zig/std/zig/system/linux.zig:112: \\processor : 0
./zig-out/lib/zig/std/zig/system/linux.zig:113: \\hart : 1
./zig-out/lib/zig/std/zig/system/linux.zig:114: \\isa : rv64imafdc
./zig-out/lib/zig/std/zig/system/linux.zig:115: \\mmu : sv39
./zig-out/lib/zig/std/zig/system/linux.zig:117: \\uarch : sifive,u74-mc
./zig-out/lib/zig/std/zig/system/linux.zig:180: \\processor : 0
./zig-out/lib/zig/std/zig/system/linux.zig:181: \\cpu : PPC970MP, altivec supported
./zig-out/lib/zig/std/zig/system/linux.zig:182: \\clock : 1250.000000MHz
./zig-out/lib/zig/std/zig/system/linux.zig:183: \\revision : 1.1 (pvr 0044 0101)
./zig-out/lib/zig/std/zig/system/linux.zig:186: \\processor : 0
./zig-out/lib/zig/std/zig/system/linux.zig:187: \\cpu : POWER8 (raw), altivec supported
./zig-out/lib/zig/std/zig/system/linux.zig:188: \\clock : 2926.000000MHz
./zig-out/lib/zig/std/zig/system/linux.zig:189: \\revision : 2.0 (pvr 004d 0200)
./zig-out/lib/zig/std/zig/system/linux.zig:307: \\processor : 0
./zig-out/lib/zig/std/zig/system/linux.zig:308: \\model name : ARMv7 Processor rev 3 (v7l)
./zig-out/lib/zig/std/zig/system/linux.zig:309: \\BogoMIPS : 18.00
./zig-out/lib/zig/std/zig/system/linux.zig:310: \\Features : half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt vfpd32 lpae
./zig-out/lib/zig/std/zig/system/linux.zig:311: \\CPU implementer : 0x41
./zig-out/lib/zig/std/zig/system/linux.zig:313: \\CPU variant : 0x0
./zig-out/lib/zig/std/zig/system/linux.zig:314: \\CPU part : 0xc07
./zig-out/lib/zig/std/zig/system/linux.zig:315: \\CPU revision : 3
./zig-out/lib/zig/std/zig/system/linux.zig:317: \\processor : 4
./zig-out/lib/zig/std/zig/system/linux.zig:318: \\model name : ARMv7 Processor rev 3 (v7l)
./zig-out/lib/zig/std/zig/system/linux.zig:319: \\BogoMIPS : 90.00
./zig-out/lib/zig/std/zig/system/linux.zig:320: \\Features : half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt vfpd32 lpae
./zig-out/lib/zig/std/zig/system/linux.zig:321: \\CPU implementer : 0x41
./zig-out/lib/zig/std/zig/system/linux.zig:323: \\CPU variant : 0x2
./zig-out/lib/zig/std/zig/system/linux.zig:324: \\CPU part : 0xc0f
./zig-out/lib/zig/std/zig/system/linux.zig:325: \\CPU revision : 3
./zig-out/lib/zig/std/zig/tokenizer.zig:1554: \\\\foo bar
./zig-out/lib/zig/std/zig/tokenizer.zig:1562: \\//foo bar
./zig-out/lib/zig/std/zig/tokenizer.zig:1563: \\//!foo bar
./zig-out/lib/zig/std/zig/tokenizer.zig:1564: \\///foo bar
./zig-out/lib/zig/std/zig/tokenizer.zig:1565: \\// foo
./zig-out/lib/zig/std/zig/tokenizer.zig:1566: \\/// foo
./zig-out/lib/zig/std/zig/tokenizer.zig:1567: \\/// /foo
./test/translate_c.zig:136: \\ typedef union {
./test/translate_c.zig:137: \\ int A;
./test/translate_c.zig:138: \\ int B;
./test/translate_c.zig:139: \\ int C;
./test/translate_c.zig:140: \\ } Foo;
./test/translate_c.zig:141: \\ Foo a = {0};
./test/translate_c.zig:142: \\ {
./test/translate_c.zig:143: \\ typedef union {
./test/translate_c.zig:144: \\ int A;
./test/translate_c.zig:145: \\ int B;
./test/translate_c.zig:146: \\ int C;
./test/translate_c.zig:147: \\ } Foo;
./test/translate_c.zig:148: \\ Foo a = {0};
./test/translate_c.zig:149: \\ }
./test/translate_c.zig:2046: \\ case 5:
./test/translate_c.zig:2050: \\ return;
./test/translate_c.zig:2057: \\ return;
./test/run_translated_c.zig:29: \\ struct foo tmp;
./test/run_translated_c.zig:33: \\ struct foo tmp;
./test/run_translated_c.zig:37: \\ bar();
./test/run_translated_c.zig:38: \\ baz();
./test/run_translated_c.zig:39: \\ return 0;
./test/run_translated_c.zig:56: \\ foo(("bar"));
./lib/std/zig/system/darwin/macos.zig:306: \\ <key>ProductBuildVersion</key>
./lib/std/zig/system/darwin/macos.zig:307: \\ <string>7W98</string>
./lib/std/zig/system/darwin/macos.zig:308: \\ <key>ProductCopyright</key>
./lib/std/zig/system/darwin/macos.zig:309: \\ <string>Apple Computer, Inc. 1983-2004</string>
./lib/std/zig/system/darwin/macos.zig:310: \\ <key>ProductName</key>
./lib/std/zig/system/darwin/macos.zig:311: \\ <string>Mac OS X</string>
./lib/std/zig/system/darwin/macos.zig:312: \\ <key>ProductUserVisibleVersion</key>
./lib/std/zig/system/darwin/macos.zig:313: \\ <string>10.3.9</string>
./lib/std/zig/system/darwin/macos.zig:314: \\ <key>ProductVersion</key>
./lib/std/zig/system/darwin/macos.zig:315: \\ <string>10.3.9</string>
./lib/std/zig/system/darwin/macos.zig:326: \\ <key>ProductBuildVersion</key>
./lib/std/zig/system/darwin/macos.zig:327: \\ <string>19G68</string>
./lib/std/zig/system/darwin/macos.zig:328: \\ <key>ProductCopyright</key>
./lib/std/zig/system/darwin/macos.zig:329: \\ <string>1983-2020 Apple Inc.</string>
./lib/std/zig/system/darwin/macos.zig:330: \\ <key>ProductName</key>
./lib/std/zig/system/darwin/macos.zig:331: \\ <string>Mac OS X</string>
./lib/std/zig/system/darwin/macos.zig:332: \\ <key>ProductUserVisibleVersion</key>
./lib/std/zig/system/darwin/macos.zig:333: \\ <string>10.15.6</string>
./lib/std/zig/system/darwin/macos.zig:334: \\ <key>ProductVersion</key>
./lib/std/zig/system/darwin/macos.zig:335: \\ <string>10.15.6</string>
./lib/std/zig/system/darwin/macos.zig:336: \\ <key>iOSSupportVersion</key>
./lib/std/zig/system/darwin/macos.zig:337: \\ <string>13.6</string>
./lib/std/zig/system/darwin/macos.zig:348: \\ <key>ProductBuildVersion</key>
./lib/std/zig/system/darwin/macos.zig:349: \\ <string>20A2408</string>
./lib/std/zig/system/darwin/macos.zig:350: \\ <key>ProductCopyright</key>
./lib/std/zig/system/darwin/macos.zig:351: \\ <string>1983-2020 Apple Inc.</string>
./lib/std/zig/system/darwin/macos.zig:352: \\ <key>ProductName</key>
./lib/std/zig/system/darwin/macos.zig:353: \\ <string>macOS</string>
./lib/std/zig/system/darwin/macos.zig:354: \\ <key>ProductUserVisibleVersion</key>
./lib/std/zig/system/darwin/macos.zig:355: \\ <string>11.0</string>
./lib/std/zig/system/darwin/macos.zig:356: \\ <key>ProductVersion</key>
./lib/std/zig/system/darwin/macos.zig:357: \\ <string>11.0</string>
./lib/std/zig/system/darwin/macos.zig:358: \\ <key>iOSSupportVersion</key>
./lib/std/zig/system/darwin/macos.zig:359: \\ <string>14.2</string>
./lib/std/zig/system/darwin/macos.zig:370: \\ <key>ProductBuildVersion</key>
./lib/std/zig/system/darwin/macos.zig:371: \\ <string>20C63</string>
./lib/std/zig/system/darwin/macos.zig:372: \\ <key>ProductCopyright</key>
./lib/std/zig/system/darwin/macos.zig:373: \\ <string>1983-2020 Apple Inc.</string>
./lib/std/zig/system/darwin/macos.zig:374: \\ <key>ProductName</key>
./lib/std/zig/system/darwin/macos.zig:375: \\ <string>macOS</string>
./lib/std/zig/system/darwin/macos.zig:376: \\ <key>ProductUserVisibleVersion</key>
./lib/std/zig/system/darwin/macos.zig:377: \\ <string>11.1</string>
./lib/std/zig/system/darwin/macos.zig:378: \\ <key>ProductVersion</key>
./lib/std/zig/system/darwin/macos.zig:379: \\ <string>11.1</string>
./lib/std/zig/system/darwin/macos.zig:380: \\ <key>iOSSupportVersion</key>
./lib/std/zig/system/darwin/macos.zig:381: \\ <string>14.3</string>
./lib/std/zig/system/linux.zig:112: \\processor : 0
./lib/std/zig/system/linux.zig:113: \\hart : 1
./lib/std/zig/system/linux.zig:114: \\isa : rv64imafdc
./lib/std/zig/system/linux.zig:115: \\mmu : sv39
./lib/std/zig/system/linux.zig:117: \\uarch : sifive,u74-mc
./lib/std/zig/system/linux.zig:180: \\processor : 0
./lib/std/zig/system/linux.zig:181: \\cpu : PPC970MP, altivec supported
./lib/std/zig/system/linux.zig:182: \\clock : 1250.000000MHz
./lib/std/zig/system/linux.zig:183: \\revision : 1.1 (pvr 0044 0101)
./lib/std/zig/system/linux.zig:186: \\processor : 0
./lib/std/zig/system/linux.zig:187: \\cpu : POWER8 (raw), altivec supported
./lib/std/zig/system/linux.zig:188: \\clock : 2926.000000MHz
./lib/std/zig/system/linux.zig:189: \\revision : 2.0 (pvr 004d 0200)
./lib/std/zig/system/linux.zig:307: \\processor : 0
./lib/std/zig/system/linux.zig:308: \\model name : ARMv7 Processor rev 3 (v7l)
./lib/std/zig/system/linux.zig:309: \\BogoMIPS : 18.00
./lib/std/zig/system/linux.zig:310: \\Features : half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt vfpd32 lpae
./lib/std/zig/system/linux.zig:311: \\CPU implementer : 0x41
./lib/std/zig/system/linux.zig:313: \\CPU variant : 0x0
./lib/std/zig/system/linux.zig:314: \\CPU part : 0xc07
./lib/std/zig/system/linux.zig:315: \\CPU revision : 3
./lib/std/zig/system/linux.zig:317: \\processor : 4
./lib/std/zig/system/linux.zig:318: \\model name : ARMv7 Processor rev 3 (v7l)
./lib/std/zig/system/linux.zig:319: \\BogoMIPS : 90.00
./lib/std/zig/system/linux.zig:320: \\Features : half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt vfpd32 lpae
./lib/std/zig/system/linux.zig:321: \\CPU implementer : 0x41
./lib/std/zig/system/linux.zig:323: \\CPU variant : 0x2
./lib/std/zig/system/linux.zig:324: \\CPU part : 0xc0f
./lib/std/zig/system/linux.zig:325: \\CPU revision : 3
./lib/std/zig/tokenizer.zig:1554: \\\\foo bar
./lib/std/zig/tokenizer.zig:1562: \\//foo bar
./lib/std/zig/tokenizer.zig:1563: \\//!foo bar
./lib/std/zig/tokenizer.zig:1564: \\///foo bar
./lib/std/zig/tokenizer.zig:1565: \\// foo
./lib/std/zig/tokenizer.zig:1566: \\/// foo
./lib/std/zig/tokenizer.zig:1567: \\/// /foo
./src/link/tapi/yaml/test.zig:242: \\are supportedI think a simple solution would be to just have the handler short circuit and not provide the code action if a carriage return is found. I've taken that approach for now.
|
WillLillis
force-pushed
the
str_code_action
branch
from
2950130
to
dd96ad0
Compare
|
Wanted to note that the expected outputs for a few of the added tests in here will have to be slightly modified if ziglang/zig#19299 is accepted. |
WillLillis
force-pushed
the
str_code_action
branch
from
dd96ad0
to
e3e23de
Compare
|
You don't need to rebase your branches whenever a new commit is pushed to the master branch unless a conflict occured. |
Sorry about that. Will hold off going forward 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a few things:
The code actions make a best effort to make things work nicely with stuff like leading equals sign and trailing newlines, but it by no means produces correctly formatted code in many scenarios. I tried to base the instrumentation and testing code around what was already in the project, but I did have to make some a few guesses. Happy to rework/ refactor this if need be :)