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

Fix duplicate imports being generated when using completions #1820

Merged
merged 12 commits into from
Aug 15, 2024

Conversation

sezna
Copy link
Contributor

@sezna sezna commented Aug 7, 2024

Closes #1777
Closes #1775

This PR updates completions in the following ways:

  • Organizes completions.rs a bit -- it had some very, very nested code that was hard to make sense of.
  • Doesn't insert an import if a symbol has already been imported via a glob import or open
  • Doesn't insert an import if a symbol has already been imported by name exactly
  • Recognizes if the parent namespace has already been imported via an alias
  • Removes the ignore_internal_callable test, which was testing internal as if it would make things private to a single namespace, when actually internal means private to a package.

Recognizing a preexisting glob import:

Screen.Recording.2024-08-06.at.5.48.17.PM.mov

Recognizing a preexisting alias:

Screen.Recording.2024-08-06.at.6.11.01.PM.mov

sezna added 6 commits August 5, 2024 14:30
…as exact imports

Still need to insert completions in the correct place (after initial comments)

This commit closes #1777 and #1775
@sezna sezna force-pushed the alex/rework-context-finder branch from 8643870 to 0a32f63 Compare August 7, 2024 01:39
@sezna sezna force-pushed the alex/rework-context-finder branch from 0a32f63 to bf9c468 Compare August 7, 2024 01:44
@sezna sezna marked this pull request as ready for review August 7, 2024 01:48
Copy link

github-actions bot commented Aug 7, 2024

Benchmark for ca9f447

Click to view benchmark
Test Base PR %
Array append evaluation 334.2±2.46µs 337.2±3.04µs +0.90%
Array literal evaluation 186.2±1.26µs 186.4±0.81µs +0.11%
Array update evaluation 415.8±3.74µs 417.3±3.31µs +0.36%
Core + Standard library compilation 23.8±0.92ms 24.0±1.04ms +0.84%
Deutsch-Jozsa evaluation 5.0±0.04ms 5.0±0.06ms 0.00%
Large file parity evaluation 34.6±0.19ms 34.6±0.26ms 0.00%
Large input file compilation 14.1±0.69ms 13.8±0.67ms -2.13%
Large input file compilation (interpreter) 54.9±2.15ms 55.1±2.25ms +0.36%
Large nested iteration 32.9±0.31ms 33.1±0.66ms +0.61%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1588.5±59.36µs 1582.3±68.76µs -0.39%
Perform Runtime Capabilities Analysis (RCA) on large file sample 8.0±0.14ms 8.0±0.15ms 0.00%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1452.2±118.80µs 1466.7±153.05µs +1.00%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 28.3±0.30ms 28.7±0.78ms +1.41%
Teleport evaluation 93.7±5.84µs 94.6±7.28µs +0.96%

Copy link

Benchmark for af48683

Click to view benchmark
Test Base PR %
Array append evaluation 337.4±3.49µs 335.7±2.16µs -0.50%
Array literal evaluation 168.0±0.82µs 168.5±2.79µs +0.30%
Array update evaluation 414.7±1.32µs 416.0±3.79µs +0.31%
Core + Standard library compilation 21.3±1.14ms 21.1±0.18ms -0.94%
Deutsch-Jozsa evaluation 4.9±0.06ms 5.0±0.05ms +2.04%
Large file parity evaluation 34.5±0.40ms 34.5±0.36ms 0.00%
Large input file compilation 12.8±0.16ms 12.6±0.25ms -1.56%
Large input file compilation (interpreter) 49.3±0.78ms 49.3±0.72ms 0.00%
Large nested iteration 33.2±0.22ms 33.0±0.18ms -0.60%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1559.0±31.06µs 1558.5±36.72µs -0.03%
Perform Runtime Capabilities Analysis (RCA) on large file sample 7.7±0.06ms 7.8±0.06ms +1.30%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1416.4±30.94µs 1418.3±38.00µs +0.13%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 27.6±0.19ms 27.6±0.10ms 0.00%
Teleport evaluation 92.6±3.45µs 92.7±3.88µs +0.11%

Copy link

Benchmark for 993880e

Click to view benchmark
Test Base PR %
Array append evaluation 345.1±30.19µs 332.7±2.33µs -3.59%
Array literal evaluation 170.4±6.38µs 187.5±5.05µs +10.04%
Array update evaluation 417.9±12.05µs 412.8±1.81µs -1.22%
Core + Standard library compilation 24.8±1.05ms 24.8±0.96ms 0.00%
Deutsch-Jozsa evaluation 4.9±0.20ms 4.9±0.06ms 0.00%
Large file parity evaluation 34.2±0.20ms 34.4±0.46ms +0.58%
Large input file compilation 15.6±0.64ms 14.9±0.61ms -4.49%
Large input file compilation (interpreter) 58.9±2.90ms 56.1±1.80ms -4.75%
Large nested iteration 33.1±0.26ms 33.0±2.08ms -0.30%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1634.0±181.69µs 1632.1±176.05µs -0.12%
Perform Runtime Capabilities Analysis (RCA) on large file sample 8.3±0.28ms 8.1±0.13ms -2.41%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1494.1±155.46µs 1484.6±157.18µs -0.64%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 28.8±0.30ms 29.1±0.27ms +1.04%
Teleport evaluation 91.9±3.72µs 93.2±3.61µs +1.41%

Copy link

Benchmark for c03aa09

Click to view benchmark
Test Base PR %
Array append evaluation 454.6±4.75µs 344.1±48.82µs -24.31%
Array literal evaluation 176.1±3.61µs 197.1±6.14µs +11.93%
Array update evaluation 536.6±5.38µs 423.8±50.63µs -21.02%
Core + Standard library compilation 21.9±0.66ms 21.8±0.24ms -0.46%
Deutsch-Jozsa evaluation 5.4±0.06ms 5.0±0.07ms -7.41%
Large file parity evaluation 35.2±0.27ms 34.8±0.79ms -1.14%
Large input file compilation 13.8±0.32ms 13.7±0.15ms -0.72%
Large input file compilation (interpreter) 52.8±2.02ms 51.7±1.36ms -2.08%
Large nested iteration 45.1±0.20ms 33.2±3.64ms -26.39%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1566.4±34.72µs 1560.0±31.07µs -0.41%
Perform Runtime Capabilities Analysis (RCA) on large file sample 7.8±0.06ms 7.8±0.06ms 0.00%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1429.0±31.44µs 1417.8±28.11µs -0.78%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 27.7±0.44ms 27.5±0.17ms -0.72%
Teleport evaluation 101.2±3.55µs 93.9±4.58µs -7.21%

Copy link
Contributor

@orpuente-MS orpuente-MS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with minor comment.

language_service/src/completion.rs Outdated Show resolved Hide resolved
@sezna sezna enabled auto-merge August 15, 2024 19:44
Copy link

Benchmark for acb46c3

Click to view benchmark
Test Base PR %
Array append evaluation 340.2±1.29µs 344.9±16.91µs +1.38%
Array literal evaluation 178.3±4.25µs 175.8±4.17µs -1.40%
Array update evaluation 422.7±8.76µs 420.2±11.01µs -0.59%
Core + Standard library compilation 22.1±0.37ms 22.4±0.70ms +1.36%
Deutsch-Jozsa evaluation 5.0±0.05ms 5.0±0.12ms 0.00%
Large file parity evaluation 34.7±0.24ms 34.7±0.26ms 0.00%
Large input file compilation 13.9±0.19ms 13.8±0.19ms -0.72%
Large input file compilation (interpreter) 54.4±2.28ms 53.2±1.69ms -2.21%
Large nested iteration 33.3±0.17ms 33.1±0.28ms -0.60%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1564.9±50.13µs 1564.8±40.63µs -0.01%
Perform Runtime Capabilities Analysis (RCA) on large file sample 7.9±0.07ms 7.9±0.06ms 0.00%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1426.1±40.61µs 1429.7±46.61µs +0.25%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 27.9±1.55ms 27.9±1.67ms 0.00%
Teleport evaluation 94.3±3.73µs 94.7±5.13µs +0.42%

@sezna sezna added this pull request to the merge queue Aug 15, 2024
Merged via the queue into main with commit f02df41 Aug 15, 2024
19 checks passed
@sezna sezna deleted the alex/rework-context-finder branch August 15, 2024 20:38
github-merge-queue bot pushed a commit that referenced this pull request Aug 20, 2024
…s generation for callables (#1863)

This PR closes #1862.

I had previously touched this function in #1820. I did my best to port
it and maintain existing functionality, but there was one conditional
that I messed up, and it was around callables originating from the same
file.

This PR fixes the bug that caused. But also, this PR tries to simplify
`callable_decl_to_completion_item` so this is less likely to happen in
the future.
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

Successfully merging this pull request may close these issues.

Auto-Imports Cause Duplicate Import Statements Auto-Imports will Trigger when Unnecessary
5 participants