search

openrewrite / rewrite-analysis

OpenRewrite recipes for data flow analysis.
Apache License 2.0
8 stars 8 forks source link

Support Locating AST elements by Line/Column #41

Closed JLLeitschuh closed 8 months ago

JLLeitschuh commented 8 months ago

This allows for finding AST nodes that are flagged by other scanners to be identified within the OpenRewrite Java AST.

Unfortunately, since each printer is unique to each language, this logic as it is currently is not multi-language

Signed-off-by: Jonathan Leitschuh Jonathan.Leitschuh@gmail.com

JLLeitschuh commented 8 months ago

@timtebeek is this something that would be desired in rewrite-java or is this the appropriate home for it?

knutwannheden commented 8 months ago

I think this could go into rewrite-java. What is your use case for this?

knutwannheden commented 8 months ago

Also, I think if this is something we may also want for other source types, that would be another candidate for something like a SourceFileService as in the following PR: https://github.com/openrewrite/rewrite/pull/3817

Up until now I have been reluctant to add this, because we are still lacking use cases for it.

JLLeitschuh commented 8 months ago

What is your use case for this?

Matching up OpenRewrite AST nodes with those from security scanners, for examples SARIF files generated by CodeQL

JLLeitschuh commented 8 months ago

Do you have a better name for this utility than CoordinateLocator? LineColumnLocator was another name I considered. Thoughts?

knutwannheden commented 8 months ago

Coordinate is insofar a bit unfortunate, as this term is already used in a different situation. Maybe something with "source file offset" or "element" (referring to the LST element)? But naming things isn't my strength either 😄

jkschneider commented 8 months ago

Indeed, we already have an OpenRewrite concept of Coordinates, so the naming overlap is not good. In the IDE it is called "Go to Line" but also involves a column. So GoToLine seems reasonable to me here as well.

image

Please don't have the public API signature return FJ types. We are trying to get away from FJ.

JLLeitschuh commented 8 months ago

Please don't have the public API signature return FJ types.

Good point. I think the method type singnature was generated by GitHub copilot and I just went with it, I should have spotted that change. Will change types

JLLeitschuh commented 8 months ago

Should finding AST elements by line/column be in core or in rewrite-java? This seems like it's likely something that we'd want to be available to all languages, eventually. But maybe starting with a smaller scope initially is better? @jkschneider what are your thoughts?

JLLeitschuh commented 8 months ago

Superseded by: https://github.com/openrewrite/rewrite/pull/3829