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

Add other output formats for TreeStructureVisitor #753

Closed
matozoid opened this issue Feb 2, 2017 · 33 comments
Closed

Add other output formats for TreeStructureVisitor #753

matozoid opened this issue Feb 2, 2017 · 33 comments
Labels
Improvement Not a bug, but a way that JP can be be enhanced to work better.
Milestone

Comments

@matozoid
Copy link
Contributor

matozoid commented Feb 2, 2017

Try...

  • DOT
  • XML
  • HTML
  • yaml
  • JSON
@matozoid matozoid added Improvement Not a bug, but a way that JP can be be enhanced to work better. Easy An ideal bug for somebody new to the project to take on! labels Feb 2, 2017
@matozoid matozoid self-assigned this Feb 2, 2017
@matozoid
Copy link
Contributor Author

matozoid commented Feb 2, 2017

<root type='MethodCallExpr'>
    <arguments type='EnclosedExpr'>
        <inner type='EnclosedExpr'>
            <inner type='BinaryExpr' operator='PLUS'>
                <left type='IntegerLiteralExpr' value='1'/>
                <right type='IntegerLiteralExpr' value='1'/>
            </inner>
        </inner>
    </arguments>
    <name type='SimpleName' identifier='println'/>
    <scope type='FieldAccessExpr'>
        <name type='SimpleName' identifier='out'/>
        <scope type='NameExpr'>
            <name type='SimpleName' identifier='System'/>
        </scope>
    </scope>
</root>

@matozoid
Copy link
Contributor Author

matozoid commented Feb 2, 2017

root (BinaryExpr)
	operator: PLUS
	left (IntegerLiteralExpr)
		value: 1
	right (IntegerLiteralExpr)
		value: 1

@ryan-beckett
Copy link
Contributor

@matozoid: Is there any rhyme or reason to the XML format you've created or just an example. Before I work on this I'd like to agree upon a syntax. I'll post a sample format that covers most of the node types that we can review before I implement it.

@ryan-beckett
Copy link
Contributor

@matozoid: Also, would it suffice to add these formatters as XMLStructureVisitor, DOTStructureVisitor, etc?

@matozoid
Copy link
Contributor Author

Okay, the full story: I was working on this before, trying to get a basic framework together that would make it easy to build these things. I generated TreeStructureVisitor for that. By now I think I should have used the TreeVisitor or a custom simple recursive one. I think the user wants to specify how to output a single node, so it should have a method that receives that node, and you can do anything there. Maybe also a context or a stack for each level. The whole idea (read #754 too) is that people can quickly get an overview of what the node structure looks like. Since it doesn't have the purpose of getting processed, the format can be arbitrary.

Anyway, the current TreeStructureVisitor should be trashed.

My plan was to create a new TreeStructureVisitor and give it a callback that will create the output. Pass different callbacks, get a different output format. It currently uses extension.

Naming: "tree structure visitor" says "I visit the structure of a tree." "XML structure visitor" says "I visit the structure of XML" which it does not - so I guess it needs the word "output" or so in there somewhere?

Output formats: I think the ones above are reasonably nice. TreeStructureVisitor does not deal correctly with lists so those are not in there yet. I say: try generating a bit in a few formats, see what you think is best, and post it here. Keep in mind that we're trying to visualize the tree to human eyes :)

Last note: if one of these needs a new maven dependency, don't do it.

@ryan-beckett
Copy link
Contributor

@matozoid: Thanks for the clarification. I'll start looking into applicable patterns and get back to you.

@matozoid matozoid removed their assignment Feb 24, 2017
@ryan-beckett
Copy link
Contributor

@matozoid, on further thought I feel like this feature can get messy quickly if trying to come up with a way to allow the user to specify how to output a node. I think it'll suffice to simply supply a few outputs out of the box following how you did in TreeStructureVisitor. Perhaps I could create a factory design atop the TreeStructureVisitor, so as not to break the API. Does this sound OK?

Suppose you did try to use TreeVisitor.visitPreOrder() , which would visit the nodes in the correct order for an output feature, but then how would you, for instance, output the closing XML tag for a node? Once process() has executed, there's no going back.

@matozoid
Copy link
Contributor Author

Okay, I don't think the current TreeStructureVisitor is the way to go. We don't care which node we're printing, so we don't need a method for each node type, so we could use a much simpler tree walking algorithm. If you try adding nodelist support to the visitor you'll also notice that there's no nice way to do so. It's okay to break the API here, check the changelog.

Here's something I was playing with a while ago: https://bitbucket.org/matozoid/serious - a library that introspects an object tree and prints it as you want. Sounds like what we want, except we don't need to track circular references, and we can use the metamodel instead of introspection. Oh, did you take a look at the metamodel yet?

Damn, maybe I shouldn't have tagged this as easy :-D

@ryan-beckett
Copy link
Contributor

I'll take a look at the library and the model and get back to you. Thanks.

@matozoid
Copy link
Contributor Author

@ryan-beckett here's the branch that outputs the test stuff seen above: https://github.com/matozoid/javaparser/commits/issue_754_more_structure_output

@ryan-beckett
Copy link
Contributor

@matozoid, thanks. That's helpful. Your idea about the framework "hit" last night. I understand a bit better now. I haven't looked at the serious library yet, but I was already thinking that using reflection it should be as simple as writing an API that accepts a mapping of node type to string output, i.e.

new TreeStructureVisitor(new HashMap<String, String>() {{
    put(MethodDeclaration.class,"<MethodDeclaration>", "</MethodDeclaration>"),
    put(AnnotationDeclaration.class,"<AnnotationDeclaration>", "</AnnotationDeclaration>");
    .
    .
    .
}});

All the client would need to do is define a mapping for the output language instead of creating a full visitor implementation. Is this what you had in mind?

@matozoid
Copy link
Contributor Author

Hmmm, what I was thinking is that for showing structure, it doesn't matter what node we're looking at. It matters what it is named, what its fields are, and what kind of fields they are (simple type, node, or nodelist.)

Anyway, I think I'm trying to steer you too much. Try implementing XML and simple text output, and put the reusable stuff in a generic class, then use that to implement some others. Keep the goal in mind: letting people study the AST. Good luck :)

@matozoid matozoid removed the Easy An ideal bug for somebody new to the project to take on! label Mar 5, 2017
@matozoid
Copy link
Contributor Author

matozoid commented Apr 6, 2017

@ryan-beckett hey Ryan, are you still around?

@ryan-beckett
Copy link
Contributor

@matozoid: Yup. Just needed a few weeks to clear my head and try and learn this code base. It's ironic you've messaged considering I just pulled the project up in my browser. Makes me think you're alerted of who views the project. Anyways, I had planned on taking a stab at the implementing DOT format.

@matozoid
Copy link
Contributor Author

matozoid commented Apr 6, 2017

Cool! Don't tell anyone about the hidden webcam streamer I put in JavaParser okay?

@ryan-beckett
Copy link
Contributor

@matozoid: Lol, I won't.

@ryan-beckett
Copy link
Contributor

@matozoid: I'm going to implement the others in the way you did the XXXDump.java files. It's the most straightforward way. IMO, patterning or abstracting any more is just overkill. It'll be:

...\ast\printer\XMLPrinter.java
...\ast\printer\JSONPrinter.java
.
.
.

@matozoid
Copy link
Contributor Author

Okay, #964 was updated

@ryan-beckett
Copy link
Contributor

ryan-beckett commented Jun 18, 2017

Two questions:

  1. Format the output or leave it without unnecessary white space. I.e.:
<outer>
     <inner>
     </inner>
</outer>

vs

<outer><inner></inner></outer>

I know there are tools to format the output, but it would it be such a bad idea for it to be done automatically?

  1. Which example will suffice in terms of output. I think the second is verbose, but are there cases where that verbosity is necessary? I feel like someone manually processing the XML might appreciate a more simplistic representation, on the other hand, I feel like I may be taking away helpful data. Better to have more and waste it, than to have not enough.
<CompilationUnit>
	<ClassOrInterfaceDeclaration isInterface='false'>
		<SimpleName identifier='A'>
		</SimpleName>
		<MethodDeclaration>
			<BlockStmt>
				<ExpressionStmt>
					<VariableDeclarationExpr>
						<VariableDeclarator>
							<SimpleName identifier='a'>
							</SimpleName>
							<PrimitiveType type='INT'>
							</PrimitiveType>
						</VariableDeclarator>
						<VariableDeclarator>
							<SimpleName identifier='b'>
							</SimpleName>
							<ArrayType>
								<PrimitiveType type='INT'>
								</PrimitiveType>
							</ArrayType>
						</VariableDeclarator>
					</VariableDeclarationExpr>
				</ExpressionStmt>
			</BlockStmt>
			<VoidType>
			</VoidType>
			<SimpleName identifier='foo'>
			</SimpleName>
		</MethodDeclaration>
	</ClassOrInterfaceDeclaration>
</CompilationUnit>
<root type='CompilationUnit'>
	<types>
		<type type='ClassOrInterfaceDeclaration' isInterface='false'>
			<name type='SimpleName' identifier='A'>
			</name>
			<members>
				<member type='MethodDeclaration'>
					<body type='BlockStmt'>
						<statements>
							<statement type='ExpressionStmt'>
								<expression type='VariableDeclarationExpr'>
									<variables>
										<variable type='VariableDeclarator'>
											<name type='SimpleName' identifier='a'>
											</name>
											<type type='PrimitiveType' type='INT'>
											</type>
										</variable>
										<variable type='VariableDeclarator'>
											<name type='SimpleName' identifier='b'>
											</name>
											<type type='ArrayType'>
												<componentType type='PrimitiveType' type='INT'>
												</componentType>
											</type>
										</variable>
									</variables>
								</expression>
							</statement>
						</statements>
					</body>
					<type type='VoidType'>
					</type>
					<name type='SimpleName' identifier='foo'>
					</name>
				</member>
			</members>
		</type>
	</types>
</root>

@matozoid
Copy link
Contributor Author

  1. People request this to get an overview of the AST, just to examine it by eye. So formatting would make sense.
  2. Phew, maybe read through the original issues to see what they were after. Both are nice (although the first is missing the property names, which would make it less uhhh... informative?)

@ryan-beckett
Copy link
Contributor

ryan-beckett commented Jun 18, 2017

Yeah, essentially the first is stripped of Metadata. We'll go with the 2nd then.

@matozoid
Copy link
Contributor Author

I've put a preliminary XmlPrinter and JsonPrinter on master so people have something to play with, you should see them when you merge master.

@ryan-beckett
Copy link
Contributor

Cool. I altered your code a little so that it prints indented content. I'll submit pull requests when I finish.

@matozoid
Copy link
Contributor Author

Hey @ryan-beckett - is this one still on your name? It's been more than half a year now with not much progress...

@ryan-beckett
Copy link
Contributor

You've done XML and JSON. Are you referring to the others?

@matozoid
Copy link
Contributor Author

There's an ASCII art printer that's not done, and the others, your last message suggests you wanted to do more work on it? I pushed it into master because people wanted to use it, not because I thought it was finished.

@ryan-beckett
Copy link
Contributor

If the XML and Json versions are fine then I'm going to leave them be and work on the others. What exactly is an ASCII art printer?

@matozoid
Copy link
Contributor Author

Alright, I'll mark them final then. The ASCII art one is something like this: https://cmatskas.com/generate-ascii-folder-structures-for-windows-with-tree/

If you think XML and JSON are all we need we can also close this issue.

@ryan-beckett
Copy link
Contributor

You can leave them open a little while longer. I don't mind taking a stab at them, but I had started working on 'pointing a Problem to a node'.

@ryan-beckett
Copy link
Contributor

ryan-beckett commented Sep 17, 2017

@matozoid: I feel like the sky is the limit with HTML output. What exactly are we trying to accomplish with it? I was thinking something like this -- a simple collapsible tree view. However, we need to use Javascript. What are your thoughts? If we're going to do the HTML version, we might as well make or worthwhile, or just scrap it. I'm in favor of scrapping it honestly, because I really don't see a use case for it that XML/JSON/YAML doesn't already cover. Same goes for the ASCII art. Has someone actually requested this? Lol. The DOT format is worthwhile because it provides a diagramming aspect.

@matozoid
Copy link
Contributor Author

Skip the HTML then, this use case has more than enough code by now :-)

ASCII art was suggested by myself because it could be a quick way to output the tree in your console - but I think YAML should take care of that now, right?

@ryan-beckett
Copy link
Contributor

Cool. I'm moving on.

@matozoid
Copy link
Contributor Author

For completeness, I was also considering that #725 exists.

Not a hint to go work on that, by the way, it's not a core thing :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Not a bug, but a way that JP can be be enhanced to work better.
Projects
None yet
Development

No branches or pull requests

2 participants