Add Trystar nodes from python 3.11 #2028

Pierre-Sassoulas · 2023-02-12T21:02:41Z

Type of Changes

	Type
✓	✨ New feature

Description

Closes #1516

This is a work in progress, I never did that before. I'm creating the class on the model given in the other class although I instinctively really wanted to not use postinit and recursively rebuild nodes inside the class itself instead. Righ tnow it seems postinit do not populate the handlers property. Pushing so we can work collaboratively on it as this is one big piece of #1516

DanielNoord · 2023-02-12T21:31:35Z

What do you need help on with this PR?

Pierre-Sassoulas · 2023-02-12T21:34:26Z

Right now I don't understand why the postinit is never called and handlers never populated, but I did not have enough time to retro-engineer things yet.

Pierre-Sassoulas · 2023-02-12T21:36:00Z

There's anoother tasks for 3.11 though :)

DanielNoord · 2023-02-12T22:26:35Z

Right now I don't understand why the postinit is never called and handlers never populated, but I did not have enough time to retro-engineer things yet.

postinit needs to be called manually. It's just a convention, not something that is done automatically.

astroid/rebuilder.py

codecov · 2023-03-04T02:04:38Z

Codecov Report

Merging #2028 (47a2544) into main (857232e) will increase coverage by 0.03%.
The diff coverage is 100.00%.

❗ Current head 47a2544 differs from pull request most recent head da728ca. Consider uploading reports for the commit da728ca to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2028      +/-   ##
==========================================
+ Coverage   92.79%   92.82%   +0.03%     
==========================================
  Files          95       95              
  Lines       11013    11067      +54     
==========================================
+ Hits        10219    10273      +54     
  Misses        794      794

Flag	Coverage Δ
linux	`92.59% <100.00%> (+0.03%)`	⬆️
pypy	`88.17% <16.66%> (-0.36%)`	⬇️
windows	`92.38% <100.00%> (+0.03%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
astroid/nodes/__init__.py	`100.00% <ø> (ø)`
astroid/nodes/node_classes.py	`95.05% <100.00%> (+0.16%)`	⬆️
astroid/rebuilder.py	`98.30% <100.00%> (+0.01%)`	⬆️

Pierre-Sassoulas · 2023-03-04T09:07:56Z

Thank you for fixing everything and unblocking the path to 2.17.0 @jacobtylerwalls :D I'm going to add more tests for inference and iterating on parents.

astroid/nodes/node_classes.py

Pierre-Sassoulas · 2023-03-05T21:48:56Z

Can't test with the full test suite in pylint because of DeprecationWarning: is_standard_module() is deprecated. Use, is_stdlib_module() or module_in_path() instead but the crash is gone as expected. I say let's release astroid 2.15.0 and pylint 2.17.0 :)

Visit child nodes of TryStar Test children Avoid creating TryExcept under TryStar block range tests Proper coverage of block_range in try star node Add text for name lookup Co-authored-by: Jacob Walls <jacobtylerwalls@gmail.com>

DanielNoord

I'm late to the party, but I think the typing of the arguments is a bit too permissive. Would be good to narrow those down in a follow up so we don't make this part of the public API before releasing.

DanielNoord · 2023-03-05T22:15:08Z

astroid/nodes/node_classes.py

+    def __init__(
+        self,
+        *,
+        lineno: int | None = None,


I'm a little late to the party, but this feels incorrect. I don't think these should ever be None right?

DanielNoord · 2023-03-05T22:15:34Z

astroid/nodes/node_classes.py

+        :param end_col_offset: The end column this node appears on in the
+            source code. Note: This is after the last symbol.
+        """
+        self.body: list[NodeNG] = []


This feel like it should be SuccesfulInferenceResult maybe?

DanielNoord · 2023-03-05T22:15:46Z

astroid/nodes/node_classes.py

+        col_offset: int | None = None,
+        end_lineno: int | None = None,
+        end_col_offset: int | None = None,
+        parent: NodeNG | None = None,


Shouldn't this be LocalsDictNodeNG?

DanielNoord · 2023-03-05T22:17:52Z

astroid/nodes/node_classes.py

+    def postinit(
+        self,
+        *,
+        body: list[NodeNG] | None = None,


All of these should never be None:
https://github.com/python/typeshed/blob/9472dc10b98570633e8777accf339dcec0179da4/stdlib/_ast.pyi#L178

But why would we bother to make TryStar the only exception? Wouldn't we just batch them all up with with, etc.?

I don't really understand your comment, but I think it's better to be more restrictive where possible since we don't clearly define our public API here. There is just more options to deal with later if we ever want to refactor this.

I do like that these are keyword only!

According to the typing orelse and finalbody should also only be Statement and not NodeNG I think, but that's another issue 😓

I'm just pointing out that the current typing in this PR matches the typing for many other nodes such as TryExcept, With, If, For, etc., and doesn't meaningfully contribute to API breakgae if we're going to tighten those all in the future to have one more affected node for the time being. (In fact, it would seem weird to me to consciously fix only one at this time, which would be its own weird API asymmetry-breakage.)

Hm, I think it would have been easier to tighten them now since we're "investigating" the node anyway and make a small comment about the typing being correct somewhere.
The issue is that right now we can't know for sure that any of the typing in astroid is correct. Marking TryStart as "reviewed and actually correct" would have been a good first step I think.

@DanielNoord DanielNoord 4 minutes ago
According to the typing orelse and finalbody should also only be Statement and not NodeNG I think, but that's another issue 😓

Interesting. We should decide if we're moving forward with #1867 before delving into that, I would think. And honestly I'd still prefer fixing all affected nodes in one fell swoop. Better release story esp. if we have to tinker with constructors to not allow None, etc., which is more impactful breakage.

Fair enough!

jacobtylerwalls · 2023-03-05T22:42:17Z

Can't test with the full test suite in pylint because of DeprecationWarning: is_standard_module() is deprecated. Use, is_stdlib_module() or module_in_path() instead

I'm doing that now to double-check things. It's as simple as:

diff --git a/pyproject.toml b/pyproject.toml
index 1c8a915ac..f3972f44e 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -88,7 +88,7 @@ test = "pytest"
 testpaths = ["tests"]
 python_files = ["*test_*.py"]
 addopts = "--strict-markers"
-filterwarnings = "error"
+filterwarnings = "ignore::DeprecationWarning"
 markers = [
     "primer_stdlib: Checks for crashes and errors when running pylint on stdlib",
     "primer_external_batch_one: Checks for crashes and errors when running pylint on external libs (batch one)",

AI0867 · 2024-03-14T22:29:23Z

Astroid incorrectly infers the type of the exceptions caught by TryStar ExceptHandler:

try:
    raise ExceptionGroup("group", [TypeError("type")])
except* TypeError as not_a_type_error:
    assert isinstance(not_a_type_error, ExceptionGroup)

In fact:

try:
    raise ExceptionGroup("group1", [TypeError("1"), ExceptionGroup("group2", [TypeError("2"), TypeError("3")]), TypeError("4")])
except* TypeError as not_a_type_error:
    assert isinstance(not_a_type_error.exceptions[1], ExceptionGroup)

In except* ExceptionType as name, name will contain a value of type ExceptionGroup[UnionException], where UnionException = Union[ExceptionGroup[UnionException], ExceptionType].

jacobtylerwalls · 2024-03-14T22:56:12Z

Astroid incorrectly infers the type of the exceptions caught by TryStar ExceptHandler:
try:

    raise ExceptionGroup("group", [TypeError("type")])

except* TypeError as not_a_type_error:

    assert isinstance(not_a_type_error, ExceptionGroup)
In fact:
try:

    raise ExceptionGroup("group1", [TypeError("1"), ExceptionGroup("group2", [TypeError("2"), TypeError("3")]), TypeError("4")])

except* TypeError as not_a_type_error:

    assert isinstance(not_a_type_error.exceptions[1], ExceptionGroup)
In except* ExceptionType as name, name will contain a value of type ExceptionGroup[UnionException], where UnionException = Union[ExceptionGroup[UnionException], ExceptionType].

Thanks, can you open an issue and state the current result in addition to the expected result?

Pierre-Sassoulas added Enhancement ✨ Improvement to a component python 3.11 labels Feb 12, 2023

Pierre-Sassoulas mentioned this pull request Feb 12, 2023

TODO list for Python 3.11 support #1516

Closed

5 tasks

Pierre-Sassoulas mentioned this pull request Feb 24, 2023

Crash Building error when trying to create ast representation of module 'test' pylint-dev/pylint#8334

Closed

jacobtylerwalls reviewed Mar 3, 2023

View reviewed changes

astroid/rebuilder.py Outdated Show resolved Hide resolved

jacobtylerwalls changed the title ~~Draft: Add Trystar nodes from python 3.11~~ Add Trystar nodes from python 3.11 Mar 4, 2023

jacobtylerwalls added this to the 2.15.0 milestone Mar 4, 2023

jacobtylerwalls approved these changes Mar 4, 2023

View reviewed changes

Pierre-Sassoulas force-pushed the support-pep-654 branch from e96e496 to b51b29b Compare March 4, 2023 10:58

Pierre-Sassoulas commented Mar 4, 2023

View reviewed changes

astroid/nodes/node_classes.py Show resolved Hide resolved

work in progress trystar refs #1516

da728ca

Visit child nodes of TryStar Test children Avoid creating TryExcept under TryStar block range tests Proper coverage of block_range in try star node Add text for name lookup Co-authored-by: Jacob Walls <jacobtylerwalls@gmail.com>

Pierre-Sassoulas force-pushed the support-pep-654 branch from 47a2544 to da728ca Compare March 5, 2023 21:59

Pierre-Sassoulas merged commit 47faf97 into main Mar 5, 2023

Pierre-Sassoulas deleted the support-pep-654 branch March 5, 2023 22:13

DanielNoord reviewed Mar 5, 2023

View reviewed changes

Pierre-Sassoulas mentioned this pull request Mar 5, 2023

Bump astroid to 2.15.0, update changelog #2045

Merged

jacobtylerwalls mentioned this pull request Jul 6, 2023

Support catching multiple exceptions introduced in pep654 in python 3.11 pylint-dev/pylint#7703

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Trystar nodes from python 3.11 #2028

Add Trystar nodes from python 3.11 #2028

Pierre-Sassoulas commented Feb 12, 2023 •

edited

Loading

DanielNoord commented Feb 12, 2023

Pierre-Sassoulas commented Feb 12, 2023

Pierre-Sassoulas commented Feb 12, 2023

DanielNoord commented Feb 12, 2023

codecov bot commented Mar 4, 2023 •

edited

Loading

Pierre-Sassoulas commented Mar 4, 2023

Pierre-Sassoulas commented Mar 5, 2023

DanielNoord left a comment

DanielNoord Mar 5, 2023

DanielNoord Mar 5, 2023

DanielNoord Mar 5, 2023

DanielNoord Mar 5, 2023

jacobtylerwalls Mar 5, 2023

DanielNoord Mar 5, 2023

DanielNoord Mar 5, 2023

jacobtylerwalls Mar 5, 2023 •

edited

Loading

DanielNoord Mar 5, 2023

jacobtylerwalls Mar 5, 2023

DanielNoord Mar 5, 2023

jacobtylerwalls commented Mar 5, 2023

AI0867 commented Mar 14, 2024

jacobtylerwalls commented Mar 14, 2024

Add Trystar nodes from python 3.11 #2028

Add Trystar nodes from python 3.11 #2028

Conversation

Pierre-Sassoulas commented Feb 12, 2023 • edited Loading

Type of Changes

Description

DanielNoord commented Feb 12, 2023

Pierre-Sassoulas commented Feb 12, 2023

Pierre-Sassoulas commented Feb 12, 2023

DanielNoord commented Feb 12, 2023

codecov bot commented Mar 4, 2023 • edited Loading

Codecov Report

Pierre-Sassoulas commented Mar 4, 2023

Pierre-Sassoulas commented Mar 5, 2023

DanielNoord left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacobtylerwalls Mar 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacobtylerwalls commented Mar 5, 2023

AI0867 commented Mar 14, 2024

jacobtylerwalls commented Mar 14, 2024

Pierre-Sassoulas commented Feb 12, 2023 •

edited

Loading

codecov bot commented Mar 4, 2023 •

edited

Loading

jacobtylerwalls Mar 5, 2023 •

edited

Loading