dart-archive / angular_analyzer_plugin

WORK MOVED TO dart-lang/angular repository
https://github.com/dart-lang/angular/tree/master/angular_analyzer_plugin
68 stars 13 forks source link

Refactor: better conversion from angular_ast to our current ast #349

Open mk13 opened 7 years ago

mk13 commented 7 years ago

We take an additional step in converting what is provided by the 'angular_ast' library and converting it into our ast. This is the result of originally not using 'angular_ast' to begin with (previously used html_parser). We do a lot of heavy lifting on our end to accommodate for the previous configuration.

Consider changing the converter to do less lifting and also adding ReferenceAttribute and LetBindingAttribute instead of using a single text attribute.

This will also cause a significant change due to changes needed in the AST. This will make the code less convoluted, cleaner, and easier to understand.

MichaelRFairhurst commented 7 years ago

how do you think about making it a visitor, btw?

mk13 commented 7 years ago

Making the LetBinding into a visitor? Or improving the visitor flow to work with the two newly added AST nodes?

MichaelRFairhurst commented 7 years ago

I confess I didn't read the body of the issue, but the topic "refactor: better conversion from angular_ast to our current ast", made me think, we could have a visitor on the angular_ast structure which produces our own ast.

The changes to break up TextAttribute are definitely totally unrelated to whether we use a visitor or not. And seem like a bigger code cleanliness win.

mk13 commented 7 years ago

Ah I see what you mean. I think it would be good to find a way to create a single visitor that generates an AST that fits our plugin needs. I'll have to do some analysis to figure out exactly what we would be affecting to ensure we aren't over-eager with the visitor into an ad-hoc result that only serves us.