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

Some additional fine-tuning of annotations, fix #16

Merged
smola merged 1 commit into from
May 31, 2017
Merged

Some additional fine-tuning of annotations, fix #16

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 68 additions & 24 deletions driver/normalizer/annotation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ Missing nodes or nodes needing new features from the SDK:
Yield
YieldFrom

= PR 57:
Starred

= PR 58:
withitem

Expand All @@ -44,6 +47,11 @@ Missing nodes or nodes needing new features from the SDK:
Slice
ExtSlice

= PR 63:
Lambda
kwarg
FunctionDef.decorator_list

= PR 81:
ListComp
SetComp
Expand All @@ -52,35 +60,37 @@ Missing nodes or nodes needing new features from the SDK:
= PR 79:
arguments

=== No PR:
= PR 111:
FormattedValue (InterpolatedValue)

JoinedStr
FormattedValue
= PR 112:
AnnAssign
annotation

Starred
= PR 113:
AsyncFunctionDef (FunctionDef + async)
Await
AsyncFor (For + async)
AsyncWith (With + async)

= PR 114:
Global
Nonlocal

=== No PR:

BoolOp collapsing: needs SDK features
arguments.defaults: needs SDK features
arguments.keywords: same

GeneratorExp: check

AnnAssign (currently as assign, needs Annotation UAST)

Lambda

Global
Nonlocal

AsyncFunctionDef (FunctionDef + async)
Await
AsyncFor (For + async)
AsyncWith (With + async)

These also need SDK list-mix features:
Compare.comparators
Compare.ops
IfCondition.left
(see: https://greentreesnakes.readthedocs.io/en/latest/nodes.html#Compare)

*/

// TODO: add the "stride", third element of slices in some way once we've the index/slice roles
var AnnotationRules = On(Any).Self(
On(Not(HasInternalType(pyast.Module))).Error(errors.New("root must be Module")),
On(HasInternalType(pyast.Module)).Roles(File).Descendants(
Expand Down Expand Up @@ -138,9 +148,10 @@ var AnnotationRules = On(Any).Self(
On(HasInternalType(pyast.NumLiteral)).Roles(NumberLiteral),
On(HasInternalType(pyast.Str)).Roles(StringLiteral),
On(HasInternalType(pyast.BoolLiteral)).Roles(BooleanLiteral),
// FIXME: JoinedStr are the fstrings (f"my name is {name}"), they have a composite AST
// with a body that is a list of StringLiteral + FormattedValue(value, conversion, format_spec)
On(HasInternalType(pyast.JoinedStr)).Roles(StringLiteral),
On(HasInternalType(pyast.JoinedStr)).Roles(StringLiteral).Children(
// FIXME: should be StringInterpolatedExpression or something like that
On(HasInternalType(pyast.FormattedValue)).Roles(Expression),
),
On(HasInternalType(pyast.NoneLiteral)).Roles(NullLiteral),
On(HasInternalType(pyast.Set)).Roles(SetLiteral),
On(HasInternalType(pyast.List)).Roles(ListLiteral),
Expand Down Expand Up @@ -225,7 +236,7 @@ var AnnotationRules = On(Any).Self(
On(HasInternalRole("handlers")).Roles(TryCatch),
On(HasInternalRole("orelse")).Roles(IfElse),
),
On(HasInternalType(pyast.TryExcept)).Roles(TryCatch), // py2
On(HasInternalType(pyast.TryExcept)).Roles(TryCatch), // py2
On(HasInternalType(pyast.ExceptHandler)).Roles(TryCatch), // py3
On(HasInternalType(pyast.TryFinally)).Roles(TryFinally),
On(HasInternalType(pyast.Raise)).Roles(Throw),
Expand All @@ -236,10 +247,27 @@ var AnnotationRules = On(Any).Self(
On(HasInternalType(pyast.Return)).Roles(Return),
On(HasInternalType(pyast.Break)).Roles(Break),
On(HasInternalType(pyast.Continue)).Roles(Continue),
// FIXME: IfCondition bodies in Python take the form:
// 1 < a < 10
// - left (internalRole): 1 (first element)
// - Compare.ops (internalType): [LessThan, LessThan]
// - Compare.comparators (internalType): ['a', 10]
// The current mapping is:
// - left: BinaryExpressionLeft
// - Compare.ops: BinaryExpressionOp
// - Compare.comparators: BinaryExpressionRight

@abeaumont abeaumont May 30, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if we need these Binary* roles at all (just thinking aloud, not related specifically to this PR). Maybe annotating operators and operands is enough and is more general (no need to make a special case for binary operators, even if they should work as well for binary operators).

@juanjux juanjux May 30, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Mmm it's pretty common to call left and right the elements of a binary expression (just like lvalue and rvalue on assignments), they could be inferred by the order or called first/second.

Your tough about generalizing UnaryExpressions, BinaryExpressions, etc to NExpressions or something like that makes a lot of sense.

@abeaumont abeaumont May 30, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, they're usually called LHS and RHS. Still that info could be implicit, not explicit, without loss of semantic information and reducing Roles pollution.

// But this is obviously not correct. To fix this properly we would need
// and SDK feature to mix lists (also needed for default and keyword arguments and
// boolean operators).
// "If that sounds awkward is because it is" (their words)
On(HasInternalType(pyast.If)).Roles(If).Children(
On(HasInternalType("If.body")).Roles(IfBody),
On(HasInternalType(pyast.Compare)).Roles(IfCondition),
On(HasInternalType("If.orelse")).Roles(IfElse),
On(HasInternalType(pyast.Compare)).Roles(IfCondition, BinaryExpression).Children(
On(HasInternalType("Compare.ops")).Roles(BinaryExpressionOp),
On(HasInternalType("Compare.comparators")).Roles(BinaryExpressionRight),
On(HasInternalRole("left")).Roles(BinaryExpressionLeft),
),
),
On(HasInternalType(pyast.IfExp)).Roles(If, Expression).Children(
// These are used on ifexpressions (a = 1 if x else 2)
Expand Down Expand Up @@ -296,11 +324,27 @@ var AnnotationRules = On(Any).Self(
// information by themselves and this we consider it comments (some preprocessors or linters can use
// them, the runtimes ignore them). The TOKEN will take the annotation in the UAST node so
// the information is keept in any case.
// FIXME: change to Annotation when PR 112 is merged
On(HasInternalRole("annotation")).Roles(Comment),
On(HasInternalRole("returns")).Roles(Comment),

// Python very odd ellipsis operator. Has a special rule in tonoder synthetic tokens
// map to load it with the token "PythonEllipsisOperator" and gets the role SimpleIdentifier
On(HasInternalType(pyast.Ellipsis)).Roles(SimpleIdentifier),

// List/Map/Set comprehensions. We map the "for x in y" to ForEach roles and the
// "if something" to If* roles. FIXME: missing the top comprehension roles in the UAST, change
// once they've been merged
On(HasInternalType(pyast.Comprehension)).Roles(ForEach).Children(
On(HasInternalRole("iter")).Roles(ForUpdate),
On(HasInternalRole("target")).Roles(ForExpression),
// FIXME: see the comment on IfCondition above
On(HasInternalType(pyast.Compare)).Roles(IfCondition, BinaryExpression).Children(
On(HasInternalType("Compare.ops")).Roles(BinaryExpressionOp),
On(HasInternalType("Compare.comparators")).Roles(BinaryExpressionRight),
On(HasInternalRole("left")).Roles(BinaryExpressionLeft),
),
),

),
)
11 changes: 0 additions & 11 deletions driver/normalizer/annotation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
package normalizer

import (
"bytes"
"encoding/json"
"fmt"
"os"
"path/filepath"
"testing"
Expand Down Expand Up @@ -53,11 +51,6 @@ func TestAnnotatePrettyAnnotationsOnly(t *testing.T) {

err = AnnotationRules.Apply(n)
require.NoError(err)

//buf := bytes.NewBuffer(nil)
//err = uast.Pretty(n, buf, uast.IncludeAnnotations|uast.IncludeChildren|uast.IncludeTokens)
//require.NoError(err)
//fmt.Println(buf.String())
}

func TestNodeTokens(t *testing.T) {
Expand All @@ -73,10 +66,8 @@ func TestNodeTokens(t *testing.T) {
tokens := uast.Tokens(n)
require.True(len(tokens) > 0)
//func Pretty(n *Node, w io.Writer, includes IncludeFlag) error {
buf := bytes.NewBuffer(nil)
err = uast.Pretty(n, os.Stdout, uast.IncludeAll)
require.NoError(err)
fmt.Println(buf.String())
}

func TestAll(t *testing.T) {
Expand All @@ -96,10 +87,8 @@ func TestAll(t *testing.T) {
require.NoError(err)

//func Pretty(n *Node, w io.Writer, includes IncludeFlag) error {
buf := bytes.NewBuffer(nil)
err = uast.Pretty(n, os.Stdout, uast.IncludeAll)
require.NoError(err)
fmt.Println(buf.String())
}

func getFixture(name string) (map[string]interface{}, error) {
Expand Down
40 changes: 20 additions & 20 deletions driver/normalizer/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,25 +31,25 @@ var ToNoder = &native.ObjectToNoder{
"Mod": "%%",
"Pow": "**",
"AugAssign": "?=",
"BitAnd" : "&",
"BitOr" : "|",
"BitXor" : "^",
"LShift" : "<<",
"RShift" : ">>",
"Eq" : "==",
"NotEq" : "!=",
"Not" : "!",
"Lt" : "<",
"LtE" : "<=",
"Gt" : ">",
"GtE" : ">=",
"Is" : "is",
"IsNot" : "not is",
"In" : "in",
"NotIn" : "not in",
"UAdd" : "+",
"USub" : "-",
"Invert" : "~",
"BitAnd": "&",
"BitOr": "|",
"BitXor": "^",
"LShift": "<<",
"RShift": ">>",
"Eq": "==",
"NotEq": "!=",
"Not": "!",
"Lt": "<",
"LtE": "<=",
"Gt": ">",
"GtE": ">=",
"Is": "is",
"IsNot": "not is",
"In": "in",
"NotIn": "not in",
"UAdd": "+",
"USub": "-",
"Invert": "~",
},
PromoteAllPropertyLists: false,
PromotedPropertyLists: map[string]map[string]bool{
Expand All @@ -75,7 +75,7 @@ var ToNoder = &native.ObjectToNoder{
// FIXME: test[ast_type=Compare].comparators is a list?? (should be "right")
}

// UASTParserBuilder creates a parser that transform source code files into *uast.Node.
// ASTParserBuilder creates a parser that transform source code files into *uast.Node.
func UASTParserBuilder(opts driver.UASTParserOptions) (driver.UASTParser, error) {
parser, err := native.ExecParser(ToNoder, opts.NativeBin)
if err != nil {
Expand Down
2 changes: 0 additions & 2 deletions driver/normalizer/parser_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package normalizer

import (
"fmt"
"testing"

"github.com/stretchr/testify/require"
Expand All @@ -16,5 +15,4 @@ func TestNativeToNoder(t *testing.T) {
n, err := ToNoder.ToNode(f)
require.NoError(err)
require.NotNil(n)
fmt.Println("NODE", n)
}
Loading