Skip to content

Commit aa7731d

Browse files
author
mgricken
committed
Fixes bug 2738438: Saving File with class, but no Identifier throws Exception
Improves detection of empty files that trip up unit testing: If the document doesn't produce a *.class file, then this will be considered "out of sync" even though it isn't. We don't really have many options here unless we actually parse the document. As a heuristic, we check if there is any non-comment text that includes the words "class", "interface" or "enum". If the file doesn't, we presume it is empty and doesn't generate a class file; therefore, it cannot ever be out of sync. Improved error diagnostics if classes are out of sync before JUnit: The documents in question are now actually displayed, and the error messages are more precise in case compiling fails. Moved MainFrame.setPopupLoc to Utilities.setPopupLoc. It doesn't have anything to do with MainFrame. git-svn-id: file:///tmp/test-svn/trunk@4872 fe72c1cf-3628-48e9-8b72-1c46755d3cff
1 parent 51e686b commit aa7731d

40 files changed

Lines changed: 831 additions & 593 deletions

‎drjava/src/edu/rice/cs/drjava/model/AbstractGlobalModel.java‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2150,17 +2150,38 @@ public List<OpenDefinitionsDocument> getOpenDefinitionsDocuments() {
21502150

21512151
/* Returns a sorted (by time of insertion) collection of all open documents. */
21522152
public List<OpenDefinitionsDocument> getSortedOpenDefinitionsDocuments() { return getOpenDefinitionsDocuments(); }
2153+
2154+
/** @return true if all open documents are in sync with their primary class files. */
2155+
public boolean hasOutOfSyncDocuments() { return getOutOfSyncDocuments().size()>0; }
2156+
2157+
public boolean hasOutOfSyncDocuments(List<OpenDefinitionsDocument> lod) { return getOutOfSyncDocuments().size()>0; }
21532158

21542159
/** @return true if all open documents are in sync with their primary class files. */
2155-
public boolean hasOutOfSyncDocuments() { return hasOutOfSyncDocuments(getOpenDefinitionsDocuments()); }
2160+
public List<OpenDefinitionsDocument> getOutOfSyncDocuments() { return getOutOfSyncDocuments(getOpenDefinitionsDocuments()); }
21562161

2157-
public boolean hasOutOfSyncDocuments(List<OpenDefinitionsDocument> lod) {
2162+
public List<OpenDefinitionsDocument> getOutOfSyncDocuments(List<OpenDefinitionsDocument> lod) {
2163+
List<OpenDefinitionsDocument> outOfSync = new ArrayList<OpenDefinitionsDocument>();
21582164
for (OpenDefinitionsDocument doc: lod) {
21592165
if (doc.isSourceFile() &&
21602166
(! isProjectActive() || doc.inProjectPath() || doc.isAuxiliaryFile()) &&
2161-
(! doc.checkIfClassFileInSync())) return true;
2167+
(! doc.checkIfClassFileInSync())) {
2168+
// If the document doesn't produce a *.class file, then this will be considered "out-of-sync",
2169+
// even though it isn't. We don't really have many options here unless we actually parse the
2170+
// document.
2171+
// As a heuristic, we check if there is any non-comment text that includes the words "class",
2172+
// "interface" or "enum". If the file doesn't, we presume it is empty and doesn't generate
2173+
// a class file; therefore, it cannot ever be out of sync.
2174+
try {
2175+
boolean b = doc.containsClassOrInterfaceOrEnum();
2176+
System.out.println("Checking contents of "+doc+": "+b);
2177+
if (b) outOfSync.add(doc);
2178+
}
2179+
catch(BadLocationException e) {
2180+
outOfSync.add(doc);
2181+
}
2182+
}
21622183
}
2163-
return false;
2184+
return outOfSync;
21642185
}
21652186

21662187
/** Set the indent tab size for all definitions documents.
@@ -3704,6 +3725,12 @@ public int getOffsetOfLine(int line) {
37043725
// /** Remove a manager for find results from this document.
37053726
// * @param rm the global model's region manager. */
37063727
// public void removeFindResultsManager(RegionManager<MovingDocumentRegion> rm) { _findResultsManagers.remove(rm); }
3728+
3729+
/** Returns true if one of the words 'class', 'interface' or 'enum' is found
3730+
* in non-comment text. */
3731+
public boolean containsClassOrInterfaceOrEnum() throws BadLocationException {
3732+
return getDocument().containsClassOrInterfaceOrEnum();
3733+
}
37073734
} /* End of ConcreteOpenDefDoc */
37083735

37093736
private static class TrivialFSS implements FileSaveSelector {

‎drjava/src/edu/rice/cs/drjava/model/DummyGlobalModel.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,14 @@ public boolean hasOutOfSyncDocuments() {
497497
public boolean hasOutOfSyncDocuments(List<OpenDefinitionsDocument> lod) {
498498
throw new UnsupportedOperationException("Tried to call hasOutOfSyncDocuments on a Dummy");
499499
}
500+
501+
public List<OpenDefinitionsDocument> getOutOfSyncDocuments() {
502+
throw new UnsupportedOperationException("Tried to call getOutOfSyncDocuments on a Dummy");
503+
}
504+
505+
public List<OpenDefinitionsDocument> getOutOfSyncDocuments(List<OpenDefinitionsDocument> lod) {
506+
throw new UnsupportedOperationException("Tried to call getOutOfSyncDocuments on a Dummy");
507+
}
500508

501509
public void cleanBuildDirectory(){
502510
throw new UnsupportedOperationException("Tried to call cleanBuildDirectory on a Dummy");

‎drjava/src/edu/rice/cs/drjava/model/DummyGlobalModelListener.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ public void filePathContainsPound() { }
102102
/** Called to demand that all class file must be in sync before running unit tests. It is up to the caller of this
103103
* method to check if the documents are out of sync, using OpenDefinitionsDocument.checkIfClassFileInSync().
104104
*/
105-
public void compileBeforeJUnit(final CompilerListener l) { }
105+
public void compileBeforeJUnit(final CompilerListener l, List<OpenDefinitionsDocument> outOfSync) { }
106106

107107
/** Called after JUnit is started by the GlobalModel. */
108108
public void junitStarted() { }
@@ -156,8 +156,9 @@ public void currentDirectoryChanged(File dir) { }
156156

157157
/** Called when trying to test a non-TestCase class.
158158
* @param isTestAll whether or not it was a use of the test all button
159+
* @param didCompileFail whether or not a compile before this JUnit attempt failed
159160
*/
160-
public void nonTestCase(boolean isTestAll) { }
161+
public void nonTestCase(boolean isTestAll, boolean didCompileFail) { }
161162

162163
/** Called when trying to test an illegal class file.
163164
* @param e the ClassFileError thrown when DrJava attempted to load the offending class.

‎drjava/src/edu/rice/cs/drjava/model/DummyOpenDefDoc.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,4 +441,5 @@ public ArrayList<HighlightStatus> getHighlightStatus(int start, int end) {
441441
public int getLineOfOffset(int offset) { throw new UnsupportedOperationException("Dummy method"); }
442442
public int getOffsetOfLine(int line) { throw new UnsupportedOperationException("Dummy method"); }
443443

444+
public boolean containsClassOrInterfaceOrEnum() throws BadLocationException { throw new UnsupportedOperationException("Dummy method"); }
444445
}

‎drjava/src/edu/rice/cs/drjava/model/GlobalEventNotifier.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -448,10 +448,11 @@ public void activeCompilerChanged() {
448448

449449
/** Called when trying to test a non-TestCase class.
450450
* @param isTestAll whether or not it was a use of the test all button
451+
* @param didCompileFail whether or not a compile before this JUnit attempt failed
451452
*/
452-
public void nonTestCase(boolean isTestAll) {
453+
public void nonTestCase(boolean isTestAll, boolean didCompileFail) {
453454
_lock.startRead();
454-
try { for (GlobalModelListener l : _listeners) { l.nonTestCase(isTestAll); } }
455+
try { for (GlobalModelListener l : _listeners) { l.nonTestCase(isTestAll, didCompileFail); } }
455456
finally { _lock.endRead(); }
456457
}
457458

@@ -467,10 +468,10 @@ public void classFileError(ClassFileError e) {
467468
/** Called before attempting unit testing if tested class files are out of sync, to give the user a chance to save. Do
468469
* not continue with JUnit if the user doesn't recompile!
469470
*/
470-
public void compileBeforeJUnit(final CompilerListener cl) {
471+
public void compileBeforeJUnit(final CompilerListener cl, List<OpenDefinitionsDocument> outOfSync) {
471472
// Utilities.show("compileBeforeJUnit invoked with argument " + cl + " in GlobalEventNotifier " + this);
472473
_lock.startRead();
473-
try { for (GlobalModelListener l : _listeners) { l.compileBeforeJUnit(cl); } }
474+
try { for (GlobalModelListener l : _listeners) { l.compileBeforeJUnit(cl, outOfSync); } }
474475
finally { _lock.endRead(); }
475476
}
476477

‎drjava/src/edu/rice/cs/drjava/model/GlobalModel.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,12 @@ public InteractionsScriptModel loadHistoryAsScript(FileOpenSelector selector)
495495
/** @return true iff no document in given list is out of sync with its primary class file. */
496496
public boolean hasOutOfSyncDocuments(List<OpenDefinitionsDocument> lod);
497497

498+
/** @return list of open documents that are out of sync with their primary class files. */
499+
public List<OpenDefinitionsDocument> getOutOfSyncDocuments();
500+
501+
/** @return list of open documents in given list that are out of sync with their primary class files. */
502+
public List<OpenDefinitionsDocument> getOutOfSyncDocuments(List<OpenDefinitionsDocument> lod);
503+
498504
/** Cleans the build directory. */
499505
public void cleanBuildDirectory();
500506

‎drjava/src/edu/rice/cs/drjava/model/GlobalModelJUnitTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -703,7 +703,7 @@ public void safeJUnitStaticInnerClass() throws Exception {
703703
public class JUnitCompileBeforeTestListener extends JUnitTestListener {
704704

705705
/* Method copied by _mainListener in MainFrame. */
706-
public void compileBeforeJUnit(final CompilerListener testAfterCompile) {
706+
public void compileBeforeJUnit(final CompilerListener testAfterCompile, List<OpenDefinitionsDocument> outOfSync) {
707707
// System.err.println("compileBeforeJUnit called in listener " + this);
708708
synchronized(this) { compileBeforeJUnitCount++; }
709709
// Compile all open source files

‎drjava/src/edu/rice/cs/drjava/model/GlobalModelTestCase.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -899,10 +899,10 @@ public void interpreterExited(int status) {
899899
public void consoleReset() { listenerFail("consoleReset fired unexpectedly"); }
900900
public void saveUntitled() { listenerFail("saveUntitled fired unexpectedly"); }
901901

902-
public void compileBeforeJUnit(CompilerListener cl) { compileBeforeJUnitCount++; }
902+
public void compileBeforeJUnit(CompilerListener cl, List<OpenDefinitionsDocument> outOfSync) { compileBeforeJUnitCount++; }
903903

904904
public void saveBeforeJavadoc() { listenerFail("saveBeforeJavadoc fired unexpectedly"); }
905-
public void nonTestCase(boolean isTestAll) { listenerFail("nonTestCase fired unexpectedly"); }
905+
public void nonTestCase(boolean isTestAll, boolean didCompileFail) { listenerFail("nonTestCase fired unexpectedly"); }
906906

907907
public boolean canAbandonFile(OpenDefinitionsDocument doc) {
908908
listenerFail("canAbandonFile fired unexpectedly");
@@ -1228,7 +1228,7 @@ public void resetJUnitCounts() {
12281228
assertEquals("junitTestEndedCount should be same as junitTestStartedCount", junitTestEndedCount,
12291229
junitTestStartedCount);
12301230
}
1231-
@Override public void nonTestCase(boolean isTestAll) {
1231+
@Override public void nonTestCase(boolean isTestAll, boolean didCompileFail) {
12321232
if (printMessages) System.out.println("listener.nonTestCase, isTestAll=" + isTestAll);
12331233
synchronized(this) { nonTestCaseCount++; }
12341234
_log.log("nonTestCase() called; notifying JUnitDone");
@@ -1254,7 +1254,7 @@ public static class JUnitNonTestListener extends JUnitTestListener {
12541254
private volatile boolean _shouldBeTestAll;
12551255
public JUnitNonTestListener() { this(false); }
12561256
public JUnitNonTestListener(boolean shouldBeTestAll) { _shouldBeTestAll = shouldBeTestAll; }
1257-
public void nonTestCase(boolean isTestAll) {
1257+
public void nonTestCase(boolean isTestAll, boolean didCompileFail) {
12581258
synchronized(this) { nonTestCaseCount++; }
12591259
assertEquals("Non test case heard the wrong value for test current/test all", _shouldBeTestAll, isTestAll);
12601260
// Utilities.show("synchronizing on _junitLock");

‎drjava/src/edu/rice/cs/drjava/model/OpenDefinitionsDocument.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,4 +366,8 @@ public interface OpenDefinitionsDocument extends DJDocument, Finalizable<Definit
366366

367367
/** Determines if pos in document is inside a comment or a string. */
368368
public boolean isShadowed(int pos);
369+
370+
/** Returns true if one of the words 'class', 'interface' or 'enum' is found
371+
* in non-comment text. */
372+
public boolean containsClassOrInterfaceOrEnum() throws BadLocationException;
369373
}

‎drjava/src/edu/rice/cs/drjava/model/definitions/DefinitionsDocument.java‎

Lines changed: 71 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -967,31 +967,33 @@ public String getEnclosingTopLevelClassName(int pos) throws ClassNameNotFoundExc
967967
*/
968968
private String getFirstClassName(int indexOfClass, int indexOfInterface,
969969
int indexOfEnum) throws ClassNameNotFoundException {
970-
971-
if ((indexOfClass == -1) && (indexOfInterface == -1) && (indexOfEnum == -1)) throw ClassNameNotFoundException.DEFAULT;
972-
973-
// should we convert this to a sorted queue or something like that?
974-
// should we have to extend this past three keywords, it will get rather hard to maintain
975-
if ((indexOfEnum == -1) ||
976-
((indexOfClass != -1) && (indexOfClass < indexOfEnum)) ||
977-
((indexOfInterface != -1) && (indexOfInterface < indexOfEnum))) {
978-
// either "enum" not found, or "enum" found after "class" or "interface"
979-
// "enum" is irrelevant
980-
// we know that at least one of indexOfClass and indexOfInterface is != -1
981-
if ((indexOfInterface == -1) ||
982-
((indexOfClass != -1) && (indexOfClass < indexOfInterface))) {
983-
// either "interface" not found, or "interface" found after "class"
984-
return getNextIdentifier(indexOfClass + "class".length());
970+
try {
971+
if ((indexOfClass == -1) && (indexOfInterface == -1) && (indexOfEnum == -1)) throw ClassNameNotFoundException.DEFAULT;
972+
973+
// should we convert this to a sorted queue or something like that?
974+
// should we have to extend this past three keywords, it will get rather hard to maintain
975+
if ((indexOfEnum == -1) ||
976+
((indexOfClass != -1) && (indexOfClass < indexOfEnum)) ||
977+
((indexOfInterface != -1) && (indexOfInterface < indexOfEnum))) {
978+
// either "enum" not found, or "enum" found after "class" or "interface"
979+
// "enum" is irrelevant
980+
// we know that at least one of indexOfClass and indexOfInterface is != -1
981+
if ((indexOfInterface == -1) ||
982+
((indexOfClass != -1) && (indexOfClass < indexOfInterface))) {
983+
// either "interface" not found, or "interface" found after "class"
984+
return getNextIdentifier(indexOfClass + "class".length());
985+
}
986+
else {
987+
// "interface" found, and found before "class"
988+
return getNextIdentifier(indexOfInterface + "interface".length());
989+
}
985990
}
986991
else {
987-
// "interface" found, and found before "class"
988-
return getNextIdentifier(indexOfInterface + "interface".length());
989-
}
992+
// "enum" found, and found before "class" and "interface"
993+
return getNextIdentifier(indexOfEnum + "enum".length());
994+
}
990995
}
991-
else {
992-
// "enum" found, and found before "class" and "interface"
993-
return getNextIdentifier(indexOfEnum + "enum".length());
994-
}
996+
catch(IllegalStateException ise) { throw ClassNameNotFoundException.DEFAULT; }
995997
}
996998

997999
/** Gets the name of the document's main class: the document's only public class/interface or
@@ -1392,4 +1394,51 @@ protected void finalize() {
13921394
}
13931395

13941396
public String toString() { return "ddoc for " + _odd; }
1397+
1398+
/** Returns true if one of the words 'class', 'interface' or 'enum' is found
1399+
* in non-comment text. */
1400+
public boolean containsClassOrInterfaceOrEnum() throws BadLocationException {
1401+
1402+
/* */ assert Utilities.TEST_MODE || EventQueue.isDispatchThread();
1403+
int i, j;
1404+
int reducedPos = 0;
1405+
1406+
final String text = getText();
1407+
final int origLocation = _currentLocation;
1408+
try {
1409+
// Move reduced model to beginning of file
1410+
_reduced.move(-origLocation);
1411+
1412+
// Walk forward from specificed position
1413+
i = text.indexOf("class", reducedPos);
1414+
j = text.indexOf("interface", reducedPos);
1415+
if (i==-1) i = j; else if (j>=0) i = Math.min(i,j);
1416+
j = text.indexOf("enum", reducedPos);
1417+
if (i==-1) i = j; else if (j>=0) i = Math.min(i,j);
1418+
while (i > - 1) {
1419+
// Move reduced model to walker's location
1420+
_reduced.move(i - reducedPos); // reduced model points to i
1421+
reducedPos = i; // reduced model points to reducedPos
1422+
1423+
// Check if matching keyword should be ignored because it is within a comment, or quotes
1424+
ReducedModelState state = _reduced.getStateAtCurrent();
1425+
if (!state.equals(FREE) || _isStartOfComment(text, i) || ((i > 0) && _isStartOfComment(text, i - 1))) {
1426+
i = text.indexOf("class", reducedPos+1);
1427+
j = text.indexOf("interface", reducedPos+1);
1428+
if (i==-1) i = j; else if (j>=0) i = Math.min(i,j);
1429+
j = text.indexOf("enum", reducedPos+1);
1430+
if (i==-1) i = j; else if (j>=0) i = Math.min(i,j);
1431+
continue; // ignore match
1432+
}
1433+
else {
1434+
return true; // found match
1435+
}
1436+
}
1437+
1438+
return false;
1439+
}
1440+
finally {
1441+
_reduced.move(origLocation - reducedPos); // Restore the state of the reduced model;
1442+
}
1443+
}
13951444
}

0 commit comments

Comments
 (0)