refactor: Update function checker to use new diagnostics #590
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/diagnostics #590 +/- ##
====================================================
+ Coverage 91.73% 91.82% +0.09%
====================================================
Files 61 61
Lines 6509 6620 +111
====================================================
+ Hits 5971 6079 +108
- Misses 538 541 +3 ☔ View full report in Codecov by Sentry. |
| | from an outer scope | ||
| | | ||
| 6 | y = x + 1 | ||
| | - `y` defined here |
There was a problem hiding this comment.
is it worth sorting the code spans based on the lines they appear in the file? it looks a little bit like this is one snippet and y is defined after baz
There was a problem hiding this comment.
Yes, that's a good idea. However, we'll need to be careful because the grammar might break in some cases when reordering:
Error: Linearity violation (at $FILE:21:8)
|
19 | def test(q: qubit @owned) -> None:
20 | use(q)
21 | foo(q)
| ^ Variable `q` with linear type `qubit` cannot be borrowed
| ...
|
20 | use(q)
| - since it was already consumed here
We'd need two version of the span labels depending on which order they are outputted. I created #608 to collect these improvement ideas
| 4: def foo(): | ||
| ^^^^^^^^^^ | ||
| GuppyError: Return type must be annotated. Try adding a `-> None` annotation. | ||
| Help: Looks like `foo` doesn't return anything. Consider annotating it with `-> |
| 5 | def foo(x: bool): | ||
| | ^^^^^^^^^^^^^^^^^ | ||
| 6 | return x | ||
| | ^^^^^^^^^^^^ Return type must be annotated |
There was a problem hiding this comment.
it'd be nice if this could highlight the foo signature instead of the whole function...
Would it be better to have the message be "return type must be specified in method signature"?
There was a problem hiding this comment.
Unfortunately, the Python AST doesn't give us a span for this. Taking the first line is also not valid since you could have weird stuff like
def foo(
# No arguments, so no argument spans
) : # Closing paren on separate line + extra spaces before the colon
| 3 | | ||
| 4 | @compile_guppy | ||
| 5 | def foo(x: bool, y) -> int: | ||
| | ^ Argument type must be annotated |
There was a problem hiding this comment.
Maybe "argument requires a type annotation"?
| | | ||
| 97 | apple == orange | ||
| 98 | apple == orange | ||
| 99 | apple == orange |
| " | from an outer scope\n", | ||
| " | \n", | ||
| "4 | def outer(x: int) -> int:\n", | ||
| " | ------ `x` defined here\n", |
There was a problem hiding this comment.
It would be cool to detect overlapping spans and only print the outer once like
def outer(x: int) -> int
----- `x` defined here
def nested() -> None
x += 1
^^^^^^ Variable `x`...
but that seems tricky...
There was a problem hiding this comment.
Yes, good idea, Rust also does this! Added to #608
🤖 I have created a release *beep* *boop* --- ## [0.13.0](v0.12.2...v0.13.0) (2024-11-12) ### ⚠ BREAKING CHANGES * `prelude` module renamed to `std` ### Features * add `qubit` discard/measure methods ([#580](#580)) ([242fa44](242fa44)) * Add `SizedIter` wrapper type ([#611](#611)) ([2e9da6b](2e9da6b)) * conventional results post processing ([#593](#593)) ([db96224](db96224)) * Improve compiler diagnostics ([#547](#547)) ([90d465d](90d465d)), closes [#551](#551) [#553](#553) [#586](#586) [#588](#588) [#587](#587) [#590](#590) [#600](#600) [#601](#601) [#606](#606) * restrict result tag sizes to 256 bytes ([#596](#596)) ([4e8e00f](4e8e00f)), closes [#595](#595) ### Bug Fixes * Mock guppy decorator during sphinx builds ([#622](#622)) ([1cccc04](1cccc04)) ### Documentation * Add DEVELOPMENT.md ([#584](#584)) ([1d29d39](1d29d39)) * Fix docs build ([#639](#639)) ([bd6011c](bd6011c)) ### Miscellaneous Chores * Manually set last release commit ([#643](#643)) ([b2d569b](b2d569b)) ### Code Refactoring * rename prelude to std ([#642](#642)) ([1a68e8e](1a68e8e)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: Agustín Borgna <agustin.borgna@quantinuum.com>
No description provided.