javaparser / javaparser

Java 1-21 Parser and Abstract Syntax Tree for Java with advanced analysis functionalities.
https://javaparser.org
Other
5.39k stars 1.15k forks source link

SourceRoot has two places to set the printer #2703

Open MysterAitch opened 4 years ago

MysterAitch commented 4 years ago

Why does SourceRoot have its own copy of a field for a printer, when it also has a parser configuration option available too?

This leads to odd situations like within the generators where specifying that we wish to use lexical preserving printer must be done via BOTH the configuration AND via the source root setter method.

Personally I think it would be fine to remove the field and have get/setPrinter methods use the parser configuration, but it might need some more thought re: the implications (maybe somebody is relying on this behaviour?).

matozoid commented 4 years ago

I've wanted to clean up that whole class, tried it a few times and stopped again. I guess this particular issue is because some people want to pass the SourceRoot around without having to pass the ParserConfiguration too? It makes sense to keep them together since SourceRoot has read and write actions that can be far apart, but have to use the same config.

On the other hand, JP embraces mutability :-)

MysterAitch commented 4 years ago

The problem I've come up against is that setting one or the other doesn't work well.

Pretty sure that reality is more complicated than this but from what I can gather, lexical preservation applies only after it is "setup" as changes to the AST need to be logged (or "observed"). Would need to go back and step through again to recall for sure.

Whatever actually happens, I can only confirm for sure that setting it via the parser config (passed via the constructor to SourceRoot) OR via SourceRoot#setPrinter() then the generators have different outputs/behaviours.

mindajalaj commented 4 years ago

Hi @MysterAitch and @matozoid , can you please share a working code snippet of Lexical preserving printer along with source root? I am trying a bunch of things couldn't get it working, sharing the code snippet here

How do we call setPrinter() on source root for Lexical preserving printer ?

    CombinedTypeSolver typeSolver = new CombinedTypeSolver(
                                        new ReflectionTypeSolver(), new JavaParserTypeSolver(codeDir));
    ParserConfiguration parserConfiguration = new ParserConfiguration()
                                                .setSymbolResolver(new JavaSymbolSolver(typeSolver));

    parserConfiguration.setLexicalPreservationEnabled(true);
    // Parse all source files
    SourceRoot sourceRoot = new SourceRoot(codeDir.toPath());
    sourceRoot.setParserConfiguration(parserConfiguration);

    List<ParseResult<CompilationUnit>> parseResults;
    try {
        parseResults = sourceRoot.tryToParse("");
        // Now get all compilation units
        compilations = parseResults.stream()
                            .filter(ParseResult::isSuccessful)
                            .map(r -> r.getResult().get())
                            .collect(Collectors.toList());

    } catch (IOException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }
    for (int i =0; i < compilations.size(); i++) {    
        CompilationUnit cuu = compilations.get(i);
        LexicalPreservingPrinter.setup(cuu);    
    }
    modifyCode(compilations);
   //sourceRoot.setPrinter(new PrettyPrinter(new PrettyPrinterConfiguration().setEndOfLineCharacter("\n"))::print);
   sourceRoot.saveAll();`
jlerbsc commented 4 years ago

Hi @mindajalaj internaly SourceRoot use a printer to print CompilationUnit. printer is defined by Function<CompilationUnit, String>. So you can create a function which use LexicalPreservingPrinter to return a String.

sourceRoot.setPrinter(cu -> LexicalPreservingPrinter.print(cu)); //this example has not been tested