From dbb3eaf4b352a2f45437cf3af2ec96274fdc2f15 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Tue, 20 Jul 2010 13:00:04 +0400 Subject: [PATCH] PSI builder: more consistent validity checks --- .../intellij/lang/impl/PsiBuilderImpl.java | 80 +++++++++---------- .../intellij/lang/LightPsiBuilderTest.java | 64 +++++++++++++++ 2 files changed, 101 insertions(+), 43 deletions(-) diff --git a/platform/lang-impl/src/com/intellij/lang/impl/PsiBuilderImpl.java b/platform/lang-impl/src/com/intellij/lang/impl/PsiBuilderImpl.java index fd614033652e..7a55ed3f5969 100644 --- a/platform/lang-impl/src/com/intellij/lang/impl/PsiBuilderImpl.java +++ b/platform/lang-impl/src/com/intellij/lang/impl/PsiBuilderImpl.java @@ -569,15 +569,7 @@ public class PsiBuilderImpl extends UserDataHolderBase implements PsiBuilder { @SuppressWarnings({"SuspiciousMethodCalls"}) public void doneBefore(Marker marker, Marker before) { -// TODO: there could be not done markers after 'marker' and that's normal - if (((StartMarker)marker).myDoneMarker != null) { - LOG.error("Marker already done."); - } - - int idx = myProduction.lastIndexOf(marker); - if (idx < 0) { - LOG.error("Marker never been added."); - } + doValidityChecks(marker, before); int beforeIndex = myProduction.lastIndexOf(before); @@ -599,7 +591,7 @@ public class PsiBuilderImpl extends UserDataHolderBase implements PsiBuilder { } public void error(Marker marker, String message) { - doValidityChecks(marker); + doValidityChecks(marker, null); DoneWithErrorMarker doneMarker = new DoneWithErrorMarker((StartMarker)marker, myCurrentLexeme, message); ((StartMarker)marker).myDoneMarker = doneMarker; @@ -608,27 +600,18 @@ public class PsiBuilderImpl extends UserDataHolderBase implements PsiBuilder { @SuppressWarnings({"SuspiciousMethodCalls"}) public void errorBefore(Marker marker, String message, Marker before) { -// TODO: there could be not done markers after 'marker' and that's normal - if (((StartMarker)marker).myDoneMarker != null) { - LOG.error("Marker already done."); - } - - int idx = myProduction.lastIndexOf(marker); - if (idx < 0) { - LOG.error("Marker has never been added."); - } + doValidityChecks(marker, before); int beforeIndex = myProduction.lastIndexOf(before); DoneWithErrorMarker doneMarker = new DoneWithErrorMarker((StartMarker)marker, myCurrentLexeme, message); doneMarker.myLexemeIndex = ((StartMarker)before).myLexemeIndex; - ((StartMarker)marker).myDoneMarker = doneMarker; myProduction.add(beforeIndex, doneMarker); } public void done(final Marker marker) { - doValidityChecks(marker); + doValidityChecks(marker, null); DoneMarker doneMarker = DONE_MARKERS.alloc(); doneMarker.myStart = (StartMarker)marker; @@ -644,30 +627,41 @@ public class PsiBuilderImpl extends UserDataHolderBase implements PsiBuilder { } @SuppressWarnings({"UseOfSystemOutOrSystemErr", "SuspiciousMethodCalls"}) - private void doValidityChecks(final Marker marker) { - if (myDebugMode) { - final DoneMarker doneMarker = ((StartMarker)marker).myDoneMarker; - if (doneMarker != null) { - LOG.error("Marker already done."); - } - int idx = myProduction.lastIndexOf(marker); - if (idx < 0) { - LOG.error("Marker never been added."); - } + private void doValidityChecks(final Marker marker, @Nullable final Marker before) { + if (!myDebugMode) return; - for (int i = myProduction.size() - 1; i > idx; i--) { - Object item = myProduction.get(i); - if (item instanceof StartMarker) { - StartMarker otherMarker = (StartMarker)item; - if (otherMarker.myDoneMarker == null) { - final Throwable debugAllocOther = otherMarker.myDebugAllocationPosition; - final Throwable debugAllocThis = ((StartMarker)marker).myDebugAllocationPosition; - if (debugAllocOther != null) { - debugAllocThis.printStackTrace(System.err); - debugAllocOther.printStackTrace(System.err); - } - LOG.error("Another not done marker added after this one. Must be done before this."); + final DoneMarker doneMarker = ((StartMarker)marker).myDoneMarker; + if (doneMarker != null) { + LOG.error("Marker already done."); + } + int idx = myProduction.lastIndexOf(marker); + if (idx < 0) { + LOG.error("Marker has never been added."); + } + + int endIdx = myProduction.size(); + if (before != null) { + endIdx = myProduction.lastIndexOf(before); + if (endIdx < 0) { + LOG.error("'Before' marker has never been added."); + } + if (idx > endIdx) { + LOG.error("'Before' marker precedes this one."); + } + } + + for (int i = endIdx - 1; i > idx; i--) { + Object item = myProduction.get(i); + if (item instanceof StartMarker) { + StartMarker otherMarker = (StartMarker)item; + if (otherMarker.myDoneMarker == null) { + final Throwable debugAllocOther = otherMarker.myDebugAllocationPosition; + final Throwable debugAllocThis = ((StartMarker)marker).myDebugAllocationPosition; + if (debugAllocOther != null) { + debugAllocThis.printStackTrace(System.err); + debugAllocOther.printStackTrace(System.err); } + LOG.error("Another not done marker added after this one. Must be done before this."); } } } diff --git a/platform/lang-impl/testSrc/com/intellij/lang/LightPsiBuilderTest.java b/platform/lang-impl/testSrc/com/intellij/lang/LightPsiBuilderTest.java index 83ee93a92f03..111228759fff 100644 --- a/platform/lang-impl/testSrc/com/intellij/lang/LightPsiBuilderTest.java +++ b/platform/lang-impl/testSrc/com/intellij/lang/LightPsiBuilderTest.java @@ -20,9 +20,13 @@ import com.intellij.lexer.LexerBase; import com.intellij.psi.impl.DebugUtil; import com.intellij.psi.tree.IElementType; import com.intellij.psi.tree.TokenSet; +import com.sun.tools.internal.xjc.util.NullStream; import org.junit.Test; +import java.io.PrintStream; + import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; public class LightPsiBuilderTest { @@ -140,6 +144,45 @@ public class LightPsiBuilderTest { " PsiElement(DIGIT)('1')\n"); } + @Test + public void testValidityChecksOnDone() throws Exception { + doFailTest("a", + new Parser() { + public void parse(PsiBuilder builder) { + final PsiBuilder.Marker first = builder.mark(); + builder.advanceLexer(); + builder.mark(); + first.done(LETTER); + } + }); + } + + @Test + public void testValidityChecksOnDoneBefore1() throws Exception { + doFailTest("a", + new Parser() { + public void parse(PsiBuilder builder) { + final PsiBuilder.Marker first = builder.mark(); + builder.advanceLexer(); + final PsiBuilder.Marker second = builder.mark(); + second.precede(); + first.doneBefore(LETTER, second); + } + }); + } + + @Test + public void testValidityChecksOnDoneBefore2() throws Exception { + doFailTest("a", + new Parser() { + public void parse(PsiBuilder builder) { + final PsiBuilder.Marker first = builder.mark(); + builder.advanceLexer(); + final PsiBuilder.Marker second = builder.mark(); + second.doneBefore(LETTER, first); + } + }); + } private interface Parser { void parse(PsiBuilder builder); @@ -154,6 +197,27 @@ public class LightPsiBuilderTest { assertEquals(expected, DebugUtil.nodeTreeToString(root, true)); } + private static void doFailTest(final String text, final Parser parser) { + final PrintStream std = System.err; + //noinspection IOResourceOpenedButNotSafelyClosed + System.setErr(new PrintStream(new NullStream())); + try { + try { + final PsiBuilder builder = new PsiBuilderImpl(new MyTestLexer(), TokenSet.EMPTY, TokenSet.EMPTY, text); + builder.setDebugMode(true); + parser.parse(builder); + fail("should fail"); + } + catch (AssertionError e) { + //System.out.println("caught: " + e); + if ("should fail".equals(e.getMessage())) throw e; + } + } + finally { + System.setErr(std); + } + } + private static class MyTestLexer extends LexerBase { private CharSequence myBuffer = ""; private int myIndex = 0;