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

Correct node position for FuncDef nodes and fix sonar codemods #423

Merged
merged 7 commits into from
Apr 2, 2024

Conversation

clavedeluna
Copy link
Contributor

@clavedeluna clavedeluna commented Mar 29, 2024

Overview

Add custom node_position for libcst.FuncDef nodes to support codemods that change code at function level

Description

  • By default, libcst calculates all node start/end position. For simple nodes that span one line, the start and end line are the same. However, function def nodes have an end line and col position that belongs to the last line of the function, so:
# pos starts at `def` below
def func(arg):
     print(something) # pos ends here

However, for many codemods we really just care about matching the first line of the function def. So I've implemented this instead

# pos starts at `def` below
def func(arg):# pos ends here
     print(something) 

so the end position is the text length of the def ... line but index 0.

This now allows us to add include/exclude and results filtering to these codemods

Additional Details

  • I also decided to json-ify sonar_issues.json.
  • was able to dedupe code
  • future work includes handling other nodes like class def

Closes #413

Copy link

sonarcloud bot commented Mar 29, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@clavedeluna clavedeluna marked this pull request as ready for review March 29, 2024 13:41
@clavedeluna clavedeluna mentioned this pull request Apr 1, 2024
Comment on lines +54 to +62
case cst.FunctionDef():
# By default a function's position includes the entire
# function definition. Instead, we will only use the first line
# of the function definition.
params_end = self.get_metadata(PositionProvider, node.params).end
return CodeRange(
start=self.get_metadata(PositionProvider, node).start,
end=CodePosition(params_end.line, params_end.column + 1),
)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rater have a general solution instead making one for every node type. I suggest changing either includes/excludes match or node_is_selected to match match any node that begins on a given line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I agree with a more general solution like that. In that case, why do we care about start/end columns at all right now? Is there not a reason why we do column matching in match_location nearby code? If there is, then that more general solution of just matching start line doesn't make much sense.

@andrecsilva andrecsilva self-requested a review April 2, 2024 11:41
Copy link
Contributor

@andrecsilva andrecsilva left a comment

Choose a reason for hiding this comment

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

Since discussions about matching nodes are pending, I think this is ok for a temporary solution.

@clavedeluna clavedeluna added this pull request to the merge queue Apr 2, 2024
Merged via the queue into main with commit 157477d Apr 2, 2024
12 checks passed
@clavedeluna clavedeluna deleted the node-positions branch April 2, 2024 11:55
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.

fix-missing-self-or-cls is missing self.node_is_selected check
2 participants