From bf60207cc0afe2d195a154b78c05f2f77685d53b Mon Sep 17 00:00:00 2001 From: Mark Koch Date: Tue, 22 Oct 2024 17:41:54 +0100 Subject: [PATCH 1/6] fix: Use whole cell when storing source for functions in jupyter --- guppylang/definition/function.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/guppylang/definition/function.py b/guppylang/definition/function.py index f0dbb1fc..747db20d 100644 --- a/guppylang/definition/function.py +++ b/guppylang/definition/function.py @@ -241,7 +241,13 @@ def parse_py_func(f: PyFunc, sources: SourceMap) -> tuple[ast.FunctionDef, str | defn = find_ipython_def(func_ast.name) if defn is not None: file = f"<{defn.cell_name}>" - sources.add_file(file, source) + sources.add_file(file, defn.cell_source) + else: + # If we couldn't find the defining cell, just use the source code we + # got from inspect. Line numbers will be wrong, but that's the best we + # can do. + sources.add_file(file, source) + line_offset = 1 else: file = inspect.getsourcefile(f) if file is None: From db5c493f2feecd5572ce5f63982d867fba5bcc88 Mon Sep 17 00:00:00 2001 From: Mark Koch Date: Wed, 23 Oct 2024 15:46:21 +0100 Subject: [PATCH 2/6] fix: Take all spans into account when determining snippet alignment --- guppylang/diagnostic.py | 16 ++++++++++++++-- .../test_two_spans_different_lineno_lens.txt | 9 +++++++++ tests/diagnostics/test_diagnostics_rendering.py | 13 +++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 tests/diagnostics/snapshots/test_two_spans_different_lineno_lens.txt diff --git a/guppylang/diagnostic.py b/guppylang/diagnostic.py index e7aae617..86a65add 100644 --- a/guppylang/diagnostic.py +++ b/guppylang/diagnostic.py @@ -208,10 +208,16 @@ def render_diagnostic(self, diag: Diagnostic) -> None: else: span = to_span(diag.span) level = self.level_str(diag.level) + all_spans = [ + span, + *(to_span(child.span) for child in diag.children if child.span), + ] + max_lineno = max(s.end.line for s in all_spans) self.buffer.append(f"{level}: {diag.rendered_title} (at {span.start})") self.render_snippet( span, diag.rendered_span_label, + max_lineno, is_primary=True, prefix_lines=self.PREFIX_CONTEXT_LINES, ) @@ -221,6 +227,7 @@ def render_diagnostic(self, diag: Diagnostic) -> None: self.render_snippet( to_span(sub_diag.span), sub_diag.rendered_span_label, + max_lineno, is_primary=False, ) if diag.message: @@ -238,7 +245,12 @@ def render_diagnostic(self, diag: Diagnostic) -> None: ) def render_snippet( - self, span: Span, label: str | None, is_primary: bool, prefix_lines: int = 0 + self, + span: Span, + label: str | None, + max_lineno: int, + is_primary: bool, + prefix_lines: int = 0, ) -> None: """Renders the source associated with a span together with an optional label. @@ -272,7 +284,7 @@ def render_snippet( additional context. """ # Check how much space we need to reserve for the leading line numbers - ll_length = len(str(span.end.line)) + ll_length = len(str(max_lineno)) highlight_char = "^" if is_primary else "-" def render_line(line: str, line_number: int | None = None) -> None: diff --git a/tests/diagnostics/snapshots/test_two_spans_different_lineno_lens.txt b/tests/diagnostics/snapshots/test_two_spans_different_lineno_lens.txt new file mode 100644 index 00000000..09e5056a --- /dev/null +++ b/tests/diagnostics/snapshots/test_two_spans_different_lineno_lens.txt @@ -0,0 +1,9 @@ +Error: Can't compare apples with oranges (at :99:6) + | + 97 | apple == orange + 98 | apple == orange + 99 | apple == orange + | ^^ Comparison attempted here + | +100 | apple == orange + | ----- This is an apple \ No newline at end of file diff --git a/tests/diagnostics/test_diagnostics_rendering.py b/tests/diagnostics/test_diagnostics_rendering.py index aaa7ff22..2a83f685 100644 --- a/tests/diagnostics/test_diagnostics_rendering.py +++ b/tests/diagnostics/test_diagnostics_rendering.py @@ -146,6 +146,19 @@ def test_justify_line_number(snapshot, request): run_test(source, diagnostic, snapshot, request) +def test_two_spans_different_lineno_lens(snapshot, request): + @dataclass(frozen=True) + class MySubDiagnostic(Note): + span_label: ClassVar[str] = "This is an apple" + + source = 100 * "apple == orange\n" + span1 = Span(Loc(file, 99, 6), Loc(file, 99, 8)) + span2 = Span(Loc(file, 100, 0), Loc(file, 100, 5)) + diagnostic = MyError(span1) + diagnostic.add_sub_diagnostic(MySubDiagnostic(span2)) + run_test(source, diagnostic, snapshot, request) + + def test_indented(snapshot, request): source = " " * 50 + "super_apple := apple ** 2\n" source += " " * 50 + " lemon := orange - apple\n" From 772e3cf676621b338605b364943bf50f81e13bed Mon Sep 17 00:00:00 2001 From: Mark Koch Date: Wed, 23 Oct 2024 16:21:23 +0100 Subject: [PATCH 3/6] refactor: Update function checker to use new diagnostics --- examples/demo.ipynb | 17 +++-- guppylang/checker/core.py | 9 +++ guppylang/checker/func_checker.py | 63 ++++++++++++++----- tests/error/misc_errors/arg_not_annotated.err | 12 ++-- .../misc_errors/return_not_annotated.err | 14 +++-- .../return_not_annotated_none1.err | 17 +++-- .../return_not_annotated_none2.err | 18 ++++-- .../nested_errors/reassign_capture_1.err | 17 +++-- .../nested_errors/reassign_capture_2.err | 17 +++-- .../nested_errors/reassign_capture_3.err | 17 +++-- 10 files changed, 140 insertions(+), 61 deletions(-) diff --git a/examples/demo.ipynb b/examples/demo.ipynb index 96d84404..3250b352 100644 --- a/examples/demo.ipynb +++ b/examples/demo.ipynb @@ -486,13 +486,18 @@ "name": "stderr", "output_type": "stream", "text": [ - "Guppy compilation failed. Error in file :6\n", + "Error: Illegal assignment (at :6:8)\n", + " | \n", + "4 | def outer(x: int) -> int:\n", + "5 | def nested() -> None:\n", + "6 | x += 1 # Mutation of captured variable x is not allowed\n", + " | ^^^^^^ Variable `x` may not be assigned to since `x` is captured\n", + " | from an outer scope\n", + " | \n", + "4 | def outer(x: int) -> int:\n", + " | ------ `x` defined here\n", "\n", - "4: def outer(x: int) -> int:\n", - "5: def nested() -> None:\n", - "6: x += 1 # Mutation of captured variable x is not allowed\n", - " ^^^^^^\n", - "GuppyError: Variable `x` defined in an outer scope (at 4:10) may not be assigned to\n" + "Guppy compilation failed due to 1 previous error\n" ] } ], diff --git a/guppylang/checker/core.py b/guppylang/checker/core.py index 4f732b13..41b3c99e 100644 --- a/guppylang/checker/core.py +++ b/guppylang/checker/core.py @@ -7,6 +7,7 @@ from typing import ( TYPE_CHECKING, Any, + ClassVar, Generic, NamedTuple, TypeAlias, @@ -20,6 +21,7 @@ from guppylang.definition.common import DefId, Definition from guppylang.definition.ty import TypeDef from guppylang.definition.value import CallableDef +from guppylang.diagnostic import Error from guppylang.tys.builtin import ( array_type_def, bool_type_def, @@ -49,6 +51,13 @@ from guppylang.definition.struct import StructField +@dataclass(frozen=True) +class UnsupportedError(Error): + title: ClassVar[str] = "Unsupported" + span_label: ClassVar[str] = "{things} are not supported" + things: str + + #: A "place" is a description for a storage location of a local value that users #: can refer to in their program. #: diff --git a/guppylang/checker/func_checker.py b/guppylang/checker/func_checker.py index 47d59b2a..c8994afd 100644 --- a/guppylang/checker/func_checker.py +++ b/guppylang/checker/func_checker.py @@ -6,14 +6,16 @@ """ import ast -from typing import TYPE_CHECKING +from dataclasses import dataclass +from typing import TYPE_CHECKING, ClassVar from guppylang.ast_util import return_nodes_in_ast, with_loc from guppylang.cfg.bb import BB from guppylang.cfg.builder import CFGBuilder from guppylang.checker.cfg_checker import CheckedCFG, check_cfg -from guppylang.checker.core import Context, Globals, Place, Variable +from guppylang.checker.core import Context, Globals, Place, UnsupportedError, Variable from guppylang.definition.common import DefId +from guppylang.diagnostic import Error, Help, Note from guppylang.error import GuppyError from guppylang.nodes import CheckedNestedFunctionDef, NestedFunctionDef from guppylang.tys.parsing import parse_function_io_types @@ -23,6 +25,36 @@ from guppylang.tys.param import Parameter +@dataclass(frozen=True) +class IllegalAssignError(Error): + title: ClassVar[str] = "Illegal assignment" + span_label: ClassVar[str] = ( + "Variable `{var}` may not be assigned to since `{var}` is captured from an " + "outer scope" + ) + var: str + + @dataclass(frozen=True) + class DefHint(Note): + span_label: ClassVar[str] = "`{var}` defined here" + var: str + + +@dataclass(frozen=True) +class MissingAnnotationError(Error): + title: ClassVar[str] = "Missing type annotation" + span_label: ClassVar[str] = "{thing} type must be annotated" + thing: str + + @dataclass(frozen=True) + class ReturnNone(Help): + message: ClassVar[str] = ( + "Looks like `{func}` doesn't return anything. Consider annotating it with " + "`-> None`." + ) + func: str + + def check_global_func_def( func_def: ast.FunctionDef, ty: FunctionType, globals: Globals ) -> CheckedCFG[Place]: @@ -67,12 +99,9 @@ def check_nested_func_def( for v, _ in captured.values(): x = v.name if x in bb.vars.assigned: - raise GuppyError( - f"Variable `{x}` defined in an outer scope (at {{0}}) may not " - f"be assigned to", - bb.vars.assigned[x], - [v.defined_at], - ) + err = IllegalAssignError(bb.vars.assigned[x], x) + err.add_sub_diagnostic(IllegalAssignError.DefHint(v.defined_at, x)) + raise GuppyError(err) # Construct inputs for checking the body CFG inputs = [v for v, _ in captured.values()] + [ @@ -123,24 +152,24 @@ def check_signature(func_def: ast.FunctionDef, globals: Globals) -> FunctionType Guppy type.""" if len(func_def.args.posonlyargs) != 0: raise GuppyError( - "Positional-only parameters not supported", func_def.args.posonlyargs[0] + UnsupportedError(func_def.args.posonlyargs[0], "Positional-only parameters") ) if len(func_def.args.kwonlyargs) != 0: raise GuppyError( - "Keyword-only parameters not supported", func_def.args.kwonlyargs[0] + UnsupportedError(func_def.args.kwonlyargs[0], "Keyword-only parameters") ) if func_def.args.vararg is not None: - raise GuppyError("*args not supported", func_def.args.vararg) + raise GuppyError(UnsupportedError(func_def.args.vararg, "Variadic args")) if func_def.args.kwarg is not None: - raise GuppyError("**kwargs not supported", func_def.args.kwarg) + raise GuppyError(UnsupportedError(func_def.args.kwarg, "Keyword args")) if func_def.returns is None: + err = MissingAnnotationError(func_def, "Return") # TODO: Error location is incorrect if all(r.value is None for r in return_nodes_in_ast(func_def)): - raise GuppyError( - "Return type must be annotated. Try adding a `-> None` annotation.", - func_def, + err.add_sub_diagnostic( + MissingAnnotationError.ReturnNone(None, func_def.name) ) - raise GuppyError("Return type must be annotated", func_def) + raise GuppyError(err) # TODO: Prepopulate mapping when using Python 3.12 style generic functions param_var_mapping: dict[str, Parameter] = {} @@ -149,7 +178,7 @@ def check_signature(func_def: ast.FunctionDef, globals: Globals) -> FunctionType for inp in func_def.args.args: ty_ast = inp.annotation if ty_ast is None: - raise GuppyError("Argument type must be annotated", inp) + raise GuppyError(MissingAnnotationError(inp, "Argument")) input_nodes.append(ty_ast) input_names.append(inp.arg) inputs, output = parse_function_io_types( diff --git a/tests/error/misc_errors/arg_not_annotated.err b/tests/error/misc_errors/arg_not_annotated.err index 0e680b48..9102d2d9 100644 --- a/tests/error/misc_errors/arg_not_annotated.err +++ b/tests/error/misc_errors/arg_not_annotated.err @@ -1,6 +1,8 @@ -Guppy compilation failed. Error in file $FILE:5 +Error: Missing type annotation (at $FILE:5:17) + | +3 | +4 | @compile_guppy +5 | def foo(x: bool, y) -> int: + | ^ Argument type must be annotated -3: @compile_guppy -4: def foo(x: bool, y) -> int: - ^ -GuppyError: Argument type must be annotated +Guppy compilation failed due to 1 previous error diff --git a/tests/error/misc_errors/return_not_annotated.err b/tests/error/misc_errors/return_not_annotated.err index e66c67e0..9e972f9e 100644 --- a/tests/error/misc_errors/return_not_annotated.err +++ b/tests/error/misc_errors/return_not_annotated.err @@ -1,6 +1,10 @@ -Guppy compilation failed. Error in file $FILE:5 +Error: Missing type annotation (at $FILE:5:0) + | +3 | +4 | @compile_guppy +5 | def foo(x: bool): + | ^^^^^^^^^^^^^^^^^ +6 | return x + | ^^^^^^^^^^^^ Return type must be annotated -3: @compile_guppy -4: def foo(x: bool): - ^^^^^^^^^^^^^^^^^ -GuppyError: Return type must be annotated +Guppy compilation failed due to 1 previous error diff --git a/tests/error/misc_errors/return_not_annotated_none1.err b/tests/error/misc_errors/return_not_annotated_none1.err index 2798749f..0c4e5563 100644 --- a/tests/error/misc_errors/return_not_annotated_none1.err +++ b/tests/error/misc_errors/return_not_annotated_none1.err @@ -1,6 +1,13 @@ -Guppy compilation failed. Error in file $FILE:5 +Error: Missing type annotation (at $FILE:5:0) + | +3 | +4 | @compile_guppy +5 | def foo(): + | ^^^^^^^^^^ +6 | return + | ^^^^^^^^^^ Return type must be annotated -3: @compile_guppy -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 `-> +None`. + +Guppy compilation failed due to 1 previous error diff --git a/tests/error/misc_errors/return_not_annotated_none2.err b/tests/error/misc_errors/return_not_annotated_none2.err index 2798749f..b6974fc3 100644 --- a/tests/error/misc_errors/return_not_annotated_none2.err +++ b/tests/error/misc_errors/return_not_annotated_none2.err @@ -1,6 +1,14 @@ -Guppy compilation failed. Error in file $FILE:5 +Error: Missing type annotation (at $FILE:5:0) + | +3 | +4 | @compile_guppy +5 | def foo(): + | ^^^^^^^^^^ + | ... +7 | return x + | ^^^^^^^^^^^^^^^^ Return type must be annotated -3: @compile_guppy -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 `-> +None`. + +Guppy compilation failed due to 1 previous error diff --git a/tests/error/nested_errors/reassign_capture_1.err b/tests/error/nested_errors/reassign_capture_1.err index e3244d25..da2272a9 100644 --- a/tests/error/nested_errors/reassign_capture_1.err +++ b/tests/error/nested_errors/reassign_capture_1.err @@ -1,7 +1,12 @@ -Guppy compilation failed. Error in file $FILE:9 +Error: Illegal assignment (at $FILE:9:8) + | +7 | +8 | def bar() -> None: +9 | y += 2 + | ^^^^^^ Variable `y` may not be assigned to since `y` is captured + | from an outer scope + | +6 | y = x + 1 + | - `y` defined here -7: -8: def bar() -> None: -9: y += 2 - ^^^^^^ -GuppyError: Variable `y` defined in an outer scope (at 6:4) may not be assigned to +Guppy compilation failed due to 1 previous error diff --git a/tests/error/nested_errors/reassign_capture_2.err b/tests/error/nested_errors/reassign_capture_2.err index 58f44b82..a66be3e6 100644 --- a/tests/error/nested_errors/reassign_capture_2.err +++ b/tests/error/nested_errors/reassign_capture_2.err @@ -1,7 +1,12 @@ -Guppy compilation failed. Error in file $FILE:11 +Error: Illegal assignment (at $FILE:11:8) + | + 9 | if 3 > 2: +10 | z = y +11 | y = 2 + | ^^^^^ Variable `y` may not be assigned to since `y` is captured + | from an outer scope + | + 6 | y = x + 1 + | - `y` defined here -9: if 3 > 2: -10: z = y -11: y = 2 - ^^^^^ -GuppyError: Variable `y` defined in an outer scope (at 6:4) may not be assigned to +Guppy compilation failed due to 1 previous error diff --git a/tests/error/nested_errors/reassign_capture_3.err b/tests/error/nested_errors/reassign_capture_3.err index c226e435..8227921f 100644 --- a/tests/error/nested_errors/reassign_capture_3.err +++ b/tests/error/nested_errors/reassign_capture_3.err @@ -1,7 +1,12 @@ -Guppy compilation failed. Error in file $FILE:11 +Error: Illegal assignment (at $FILE:11:12) + | + 9 | +10 | def baz() -> None: +11 | y += 2 + | ^^^^^^ Variable `y` may not be assigned to since `y` is captured + | from an outer scope + | + 6 | y = x + 1 + | - `y` defined here -9: -10: def baz() -> None: -11: y += 2 - ^^^^^^ -GuppyError: Variable `y` defined in an outer scope (at 6:4) may not be assigned to +Guppy compilation failed due to 1 previous error From 184b496b5a2fafe0bd7e71c33218001a05f03e9e Mon Sep 17 00:00:00 2001 From: Mark Koch Date: Tue, 29 Oct 2024 09:39:11 +0000 Subject: [PATCH 4/6] Make new line in test clearer --- .../snapshots/test_two_spans_different_lineno_lens.txt | 4 ++-- tests/diagnostics/test_diagnostics_rendering.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/diagnostics/snapshots/test_two_spans_different_lineno_lens.txt b/tests/diagnostics/snapshots/test_two_spans_different_lineno_lens.txt index 09e5056a..d0c183a1 100644 --- a/tests/diagnostics/snapshots/test_two_spans_different_lineno_lens.txt +++ b/tests/diagnostics/snapshots/test_two_spans_different_lineno_lens.txt @@ -5,5 +5,5 @@ Error: Can't compare apples with oranges (at :99:6) 99 | apple == orange | ^^ Comparison attempted here | -100 | apple == orange - | ----- This is an apple \ No newline at end of file +100 | This apple is on another line + | ----- This is an apple \ No newline at end of file diff --git a/tests/diagnostics/test_diagnostics_rendering.py b/tests/diagnostics/test_diagnostics_rendering.py index 2a83f685..90188cba 100644 --- a/tests/diagnostics/test_diagnostics_rendering.py +++ b/tests/diagnostics/test_diagnostics_rendering.py @@ -151,9 +151,9 @@ def test_two_spans_different_lineno_lens(snapshot, request): class MySubDiagnostic(Note): span_label: ClassVar[str] = "This is an apple" - source = 100 * "apple == orange\n" + source = 99 * "apple == orange\n" + "This apple is on another line" span1 = Span(Loc(file, 99, 6), Loc(file, 99, 8)) - span2 = Span(Loc(file, 100, 0), Loc(file, 100, 5)) + span2 = Span(Loc(file, 100, 5), Loc(file, 100, 10)) diagnostic = MyError(span1) diagnostic.add_sub_diagnostic(MySubDiagnostic(span2)) run_test(source, diagnostic, snapshot, request) From d760742f76d4f9cbb6e67038f7cf7460a1ff227a Mon Sep 17 00:00:00 2001 From: Mark Koch Date: Tue, 29 Oct 2024 09:41:02 +0000 Subject: [PATCH 5/6] Use list addition --- guppylang/diagnostic.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/guppylang/diagnostic.py b/guppylang/diagnostic.py index 86a65add..e076956f 100644 --- a/guppylang/diagnostic.py +++ b/guppylang/diagnostic.py @@ -208,9 +208,8 @@ def render_diagnostic(self, diag: Diagnostic) -> None: else: span = to_span(diag.span) level = self.level_str(diag.level) - all_spans = [ - span, - *(to_span(child.span) for child in diag.children if child.span), + all_spans = [span] + [ + to_span(child.span) for child in diag.children if child.span ] max_lineno = max(s.end.line for s in all_spans) self.buffer.append(f"{level}: {diag.rendered_title} (at {span.start})") From 6a223640daf6f88147b126438bb420e52e40d8e0 Mon Sep 17 00:00:00 2001 From: Mark Koch Date: Thu, 31 Oct 2024 12:30:28 +0000 Subject: [PATCH 6/6] Reword missing annotation error --- guppylang/checker/func_checker.py | 17 +++++++++++------ tests/error/misc_errors/arg_not_annotated.err | 2 +- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/guppylang/checker/func_checker.py b/guppylang/checker/func_checker.py index c8994afd..7213de4a 100644 --- a/guppylang/checker/func_checker.py +++ b/guppylang/checker/func_checker.py @@ -41,10 +41,15 @@ class DefHint(Note): @dataclass(frozen=True) -class MissingAnnotationError(Error): +class MissingArgAnnotationError(Error): title: ClassVar[str] = "Missing type annotation" - span_label: ClassVar[str] = "{thing} type must be annotated" - thing: str + span_label: ClassVar[str] = "Argument requires a type annotation" + + +@dataclass(frozen=True) +class MissingReturnAnnotationError(Error): + title: ClassVar[str] = "Missing type annotation" + span_label: ClassVar[str] = "Return type must be annotated" @dataclass(frozen=True) class ReturnNone(Help): @@ -163,11 +168,11 @@ def check_signature(func_def: ast.FunctionDef, globals: Globals) -> FunctionType if func_def.args.kwarg is not None: raise GuppyError(UnsupportedError(func_def.args.kwarg, "Keyword args")) if func_def.returns is None: - err = MissingAnnotationError(func_def, "Return") + err = MissingReturnAnnotationError(func_def) # TODO: Error location is incorrect if all(r.value is None for r in return_nodes_in_ast(func_def)): err.add_sub_diagnostic( - MissingAnnotationError.ReturnNone(None, func_def.name) + MissingReturnAnnotationError.ReturnNone(None, func_def.name) ) raise GuppyError(err) @@ -178,7 +183,7 @@ def check_signature(func_def: ast.FunctionDef, globals: Globals) -> FunctionType for inp in func_def.args.args: ty_ast = inp.annotation if ty_ast is None: - raise GuppyError(MissingAnnotationError(inp, "Argument")) + raise GuppyError(MissingArgAnnotationError(inp)) input_nodes.append(ty_ast) input_names.append(inp.arg) inputs, output = parse_function_io_types( diff --git a/tests/error/misc_errors/arg_not_annotated.err b/tests/error/misc_errors/arg_not_annotated.err index 9102d2d9..550cd3ce 100644 --- a/tests/error/misc_errors/arg_not_annotated.err +++ b/tests/error/misc_errors/arg_not_annotated.err @@ -3,6 +3,6 @@ Error: Missing type annotation (at $FILE:5:17) 3 | 4 | @compile_guppy 5 | def foo(x: bool, y) -> int: - | ^ Argument type must be annotated + | ^ Argument requires a type annotation Guppy compilation failed due to 1 previous error