Skip to content

Create JavaParser dumpfile on FINE log level only.#3742

Closed
lkishalmi wants to merge 1 commit intoapache:masterfrom
lkishalmi:disable-javacparser-dumpfile
Closed

Create JavaParser dumpfile on FINE log level only.#3742
lkishalmi wants to merge 1 commit intoapache:masterfrom
lkishalmi:disable-javacparser-dumpfile

Conversation

@lkishalmi
Copy link
Copy Markdown
Contributor

This is a trivial PR addressing the issue on #3728. Which puts the dumpfile creation under FINE loglevel.

@lkishalmi lkishalmi added this to the NB14 milestone Mar 9, 2022
@lkishalmi lkishalmi force-pushed the disable-javacparser-dumpfile branch from e14a5ae to 22368c4 Compare March 10, 2022 15:50
@matthiasblaesing
Copy link
Copy Markdown
Contributor

The change in VanillaPartialReparser is unnessary, as the code path can only be hit if assertions are enabled (with assertions disabled (i.e. a non dev build), 332 is not hit and a stays false):

boolean a = false;
assert a = true;
if (a && LOGGER.isLoggable(Level.FINE)) {

IMHO then it is correct, that the problem is reported.

I don't think these changes address the problem reported. The reported dump files are created by VerifyPartialReparse:

private void verifyCompilationInfos(CompilationInfoImpl reparsed, CompilationInfoImpl verifyInfo) throws IOException {
if (cancel.get()) return ;
String failInfo = "";
//move to phase, and verify
if (verifyInfo.toPhase(reparsed.getPhase()) != reparsed.getPhase()) {
failInfo += "Expected phase: " + reparsed.getPhase() + ", actual phase: " + verifyInfo.getPhase();
}
if (cancel.get()) return ;
//verify diagnostics:
Set<String> reparsedDiags = (Set<String>) reparsed.getDiagnostics().stream().map(this::diagnosticToString).collect(Collectors.toSet());
if (cancel.get()) return ;
Set<String> verifyDiags = (Set<String>) verifyInfo.getDiagnostics().stream().map(this::diagnosticToString).collect(Collectors.toSet());
if (cancel.get()) return ;
if (!Objects.equals(reparsedDiags, verifyDiags)) {
failInfo += "Expected diags: " + reparsedDiags + ", actual diags: " + verifyDiags;
}
if (cancel.get()) return ;
String reparsedTree = treeToString(reparsed, reparsed.getCompilationUnit());
if (cancel.get()) return ;
String verifyTree = treeToString(verifyInfo, verifyInfo.getCompilationUnit());
if (cancel.get()) return ;
if (!Objects.equals(reparsedTree, verifyTree)) {
failInfo += "Expected tree: " + reparsedTree + "\n" + " actual tree: " + verifyTree;
}
if (!failInfo.isEmpty() && !cancel.get()) {
Utilities.revalidate(reparsed.getFileObject());
File dumpFile = JavacParser.createDumpFile(reparsed);
if (dumpFile != null) {
try (OutputStream os = new FileOutputStream(dumpFile);
PrintWriter writer = new PrintWriter(new OutputStreamWriter(os, StandardCharsets.UTF_8))) { //NOI18N
writer.println("Incorrectly reparsed file: " + reparsed.getFileObject().toURI());
Snapshot original = reparsed.getPartialReparseLastGoodSnapshot();
if (original != null) {
writer.println("----- Original text: ---------------------------------------------");
writer.println(original.getText());
};
writer.println("----- Updated text: ---------------------------------------------");
writer.println(reparsed.getSnapshot().getText());
writer.println("----- Errors: ---------------------------------------------");
writer.println(failInfo);
}
}
LOGGER.log(Level.INFO, "Incorrect partial reparse detected, dump filed: " + dumpFile);
} else {
reparsed.setPartialReparseLastGoodSnapshot(reparsed.getSnapshot());
}
}

line 534 creates the diff between the trees. As dumping is already partially protected it might be an option to tie the activation of VerifyPartialReparse to assertions:

@MimeRegistration(service=TaskFactory.class, mimeType="text/x-java")
public static final class FactoryImpl extends TaskFactory {
@Override
public Collection<? extends SchedulerTask> create(Snapshot snapshot) {
return Collections.singletonList(new VerifyPartialReparse());
}
}

 @MimeRegistration(service=TaskFactory.class, mimeType="text/x-java") 
 public static final class FactoryImpl extends TaskFactory { 
  
     @Override 
     public Collection<? extends SchedulerTask> create(Snapshot snapshot) { 
         boolean  enableVerifier = false; 
         assert enableVerifier = true; 
         if (enableVerifier) { 
             return Collections.singletonList(new VerifyPartialReparse()); 
         }
     } 
  
 } 

@jlahoda what do you think about this?

Copy link
Copy Markdown
Contributor

@jlahoda jlahoda left a comment

Choose a reason for hiding this comment

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

Ok, more or less as a stop gap measure.

@lkishalmi
Copy link
Copy Markdown
Contributor Author

Closing in favor of #3850

@lkishalmi lkishalmi closed this Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants