Skip to content

Conversation

cxzhong
Copy link
Contributor

@cxzhong cxzhong commented Oct 12, 2025

1.In Python 3.14, the AST representation was fully unified to use ast.Constant nodes for all literal values (numbers, strings, booleans, None). Previously, Python 3.8-3.13 still generated legacy node types (ast.Num, ast.Str, ast.NameConstant) for backward compatibility.

The SageArgSpecVisitor class had visit methods for the legacy node types but was missing visit_Constant(), causing it to return None for all constant default argument values when parsing function signatures on Python 3.14.

This fix adds the visit_Constant() method to properly handle the unified AST representation, allowing correct extraction of default argument values like base=0 in Integer.__init__(self, x=None, base=0).
2. In line 464, The output format is changed in python 3.14

  • Old format: <ast.Assign object at ...> contains "Assign"
  • New format: Assign(targets=[...], ...) starts with "Assign"
  1. Remove the deprecated visit methods from 3.8 and finally deleted in python 3.14.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

#41028 (part)

…SpecVisitor

In Python 3.14, the AST representation was fully unified to use ast.Constant
nodes for all literal values (numbers, strings, booleans, None). Previously,
Python 3.8-3.13 still generated legacy node types (ast.Num, ast.Str,
ast.NameConstant) for backward compatibility.

The SageArgSpecVisitor class had visit methods for the legacy node types
but was missing visit_Constant(), causing it to return None for all constant
default argument values when parsing function signatures on Python 3.14.

This fix adds the visit_Constant() method to properly handle the unified
AST representation, allowing correct extraction of default argument values
like base=0 in Integer.__init__(self, x=None, base=0).

Fixes: #<issue_number> (if applicable)
 - Old format: <ast.Assign object at ...> contains "Assign"
 - New format: Assign(targets=[...], ...) starts with "Assign"
@cxzhong cxzhong marked this pull request as ready for review October 12, 2025 14:20
@Copilot Copilot AI review requested due to automatic review settings October 12, 2025 14:20
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes compatibility issues in sageinspect for Python 3.14, addressing changes in AST representation and string formatting. Python 3.14 unified all literal values to use ast.Constant nodes and modified the output format for AST objects.

  • Adds visit_Constant() method to handle unified AST representation in Python 3.14
  • Updates test docstring to accommodate changed AST object string representation
  • Ensures proper extraction of default argument values from function signatures

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

github-actions bot commented Oct 12, 2025

Documentation preview for this PR (built with commit 95311c9; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

Removed visit_NameConstant and visit_Num methods from SageArgSpecVisitor class.
@cxzhong cxzhong requested review from Copilot and user202729 October 13, 2025 05:53
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@cxzhong
Copy link
Contributor Author

cxzhong commented Oct 13, 2025

@user202729 Can I set positive review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants