Skip to content
This repository was archived by the owner on Mar 8, 2020. It is now read-only.

Conversation

@juanjux
Copy link
Contributor

@juanjux juanjux commented Jul 6, 2017

This adds the offset to the Python nodes, using the transformation parser. It also fixes bug #35 that was a requirement for the former to work. This fixes 2 of the 3 tasks required for fixing #30.

For the next PR I will fork pydetector into bblfsh as @abeaumont suggested some time ago because it will require extensive changes.

PR includes:

- Use a TransformationParser with a FillOffset function to add the offset.

- Fix bug with some columns showing at 0.

// ASTParserBuilder creates a parser that transform source code files into *uast.Node.
func UASTParserBuilder(opts driver.UASTParserOptions) (driver.UASTParser, error) {
// transUASTParserBuilder creates a parser that transform source code files into *uast.Node
Copy link
Contributor

@abeaumont abeaumont Jul 7, 2017

Choose a reason for hiding this comment

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

Function name doesn't match with real one. I'd suggest to remove the function name from the documentation, since it's redundant and error-prone

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

@@ -1,5 +1,6 @@
Status: ok
Status: error
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this status change expected? The same question applies to the next UAST files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh f*ck I was so centered on the non zero column number and offset that skipped these ones, good catch, thanks.

Copy link
Contributor Author

@juanjux juanjux Jul 7, 2017

Choose a reason for hiding this comment

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

Updated. I added line numbers to some nodes that didn't have them in the native AST (which set them at 0 in the UAST). This fixes all of the previous errors except for the new empty.* test (expected, since it's empty, it's tests that the Python driver doesn't crash on this case like happened with a previous solved issue) and the string_fstring.*.

This last one does the curious thing of setting the line numbers the code inside the fstring's {...} as starting from line 1. Shouldn't matter much because the next PR should check and fix all those "questionable" locations of the native AST.

@juanjux juanjux changed the title Fill offset. Fix some columns with 0 value. Fill offset. Fix some linenums and columns with 0 value. Jul 7, 2017
@juanjux juanjux merged commit 34b290f into bblfsh:master Jul 10, 2017
@juanjux juanjux deleted the feature/pos_offset branch July 13, 2017 08:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants