From b3898cfb0db229736b4e8abe36680e88d8a73935 Mon Sep 17 00:00:00 2001 From: "Pavel V. Talanov" Date: Tue, 22 Apr 2014 17:58:54 +0400 Subject: [PATCH] Force the user of AstAccessControl to write at least one test violating restriction, implement such tests for existing tests This is needed because the slightest change in the test setup can make the check useless without the client knowing The solution is ugly but gives at least some protection against this --- .../ClassWithConstVal.kt | 1 - ...eFunctionWithNoSpecifiedType.dependency.kt | 3 ++ .../CompleteFunctionWithNoSpecifiedType.kt | 9 ++++ ...stractMultiFileJvmBasicCompletionTest.java | 28 +++++++---- ...tiFileJvmBasicCompletionTestGenerated.java | 5 ++ .../stubs/AbstractLazyResolveByStubTest.java | 24 ++++++++-- .../jet/plugin/stubs/AstAccessControl.kt | 47 +++++++++++++++++-- 7 files changed, 100 insertions(+), 17 deletions(-) create mode 100644 idea/testData/completion/basic/multifile/CompleteFunctionWithNoSpecifiedType/CompleteFunctionWithNoSpecifiedType.dependency.kt create mode 100644 idea/testData/completion/basic/multifile/CompleteFunctionWithNoSpecifiedType/CompleteFunctionWithNoSpecifiedType.kt diff --git a/compiler/testData/loadJava/compiledJavaCompareWithKotlin/ClassWithConstVal.kt b/compiler/testData/loadJava/compiledJavaCompareWithKotlin/ClassWithConstVal.kt index f7a59584d41..50f79015519 100644 --- a/compiler/testData/loadJava/compiledJavaCompareWithKotlin/ClassWithConstVal.kt +++ b/compiler/testData/loadJava/compiledJavaCompareWithKotlin/ClassWithConstVal.kt @@ -1,4 +1,3 @@ -// ALLOW_AST_ACCESS package test public class ClassWithConstVal() : java.lang.Object() { diff --git a/idea/testData/completion/basic/multifile/CompleteFunctionWithNoSpecifiedType/CompleteFunctionWithNoSpecifiedType.dependency.kt b/idea/testData/completion/basic/multifile/CompleteFunctionWithNoSpecifiedType/CompleteFunctionWithNoSpecifiedType.dependency.kt new file mode 100644 index 00000000000..7bd610bba89 --- /dev/null +++ b/idea/testData/completion/basic/multifile/CompleteFunctionWithNoSpecifiedType/CompleteFunctionWithNoSpecifiedType.dependency.kt @@ -0,0 +1,3 @@ +package second + +fun testFun() = 12 diff --git a/idea/testData/completion/basic/multifile/CompleteFunctionWithNoSpecifiedType/CompleteFunctionWithNoSpecifiedType.kt b/idea/testData/completion/basic/multifile/CompleteFunctionWithNoSpecifiedType/CompleteFunctionWithNoSpecifiedType.kt new file mode 100644 index 00000000000..54dfeb36366 --- /dev/null +++ b/idea/testData/completion/basic/multifile/CompleteFunctionWithNoSpecifiedType/CompleteFunctionWithNoSpecifiedType.kt @@ -0,0 +1,9 @@ +package first + +import second.testFun + +fun test() { + te +} + +// EXIST: testFun diff --git a/idea/tests/org/jetbrains/jet/completion/AbstractMultiFileJvmBasicCompletionTest.java b/idea/tests/org/jetbrains/jet/completion/AbstractMultiFileJvmBasicCompletionTest.java index 21766e3c4e1..1ade2cea641 100644 --- a/idea/tests/org/jetbrains/jet/completion/AbstractMultiFileJvmBasicCompletionTest.java +++ b/idea/tests/org/jetbrains/jet/completion/AbstractMultiFileJvmBasicCompletionTest.java @@ -18,24 +18,36 @@ package org.jetbrains.jet.completion; import com.intellij.codeInsight.completion.CompletionTestCase; import com.intellij.codeInsight.lookup.LookupElement; +import kotlin.Function0; import kotlin.Function1; +import kotlin.Unit; import org.jetbrains.annotations.NotNull; import org.jetbrains.jet.completion.util.UtilPackage; import org.jetbrains.jet.plugin.PluginTestCaseBase; import org.jetbrains.jet.plugin.project.TargetPlatform; +import org.jetbrains.jet.plugin.stubs.AstAccessControl; public abstract class AbstractMultiFileJvmBasicCompletionTest extends CompletionTestCase { protected void doTest(@NotNull String testPath) throws Exception { configureByFile(getTestName(false) + ".kt", ""); - UtilPackage.testCompletion(getFile().getText(), TargetPlatform.JVM, new Function1() { - @Override - public LookupElement[] invoke(Integer invocationCount) { - complete(invocationCount); - return myItems; - } - }); - + boolean shouldFail = testPath.contains("NoSpecifiedType"); + AstAccessControl.instance$.testWithControlledAccessToAst( + shouldFail, getFile().getVirtualFile(), getProject(), getTestRootDisposable(), + new Function0() { + @Override + public Unit invoke() { + UtilPackage.testCompletion(getFile().getText(), TargetPlatform.JVM, new Function1() { + @Override + public LookupElement[] invoke(Integer invocationCount) { + complete(invocationCount); + return myItems; + } + }); + return Unit.VALUE; + } + } + ); } @Override diff --git a/idea/tests/org/jetbrains/jet/completion/MultiFileJvmBasicCompletionTestGenerated.java b/idea/tests/org/jetbrains/jet/completion/MultiFileJvmBasicCompletionTestGenerated.java index 36a6788fd70..38ae7b78298 100644 --- a/idea/tests/org/jetbrains/jet/completion/MultiFileJvmBasicCompletionTestGenerated.java +++ b/idea/tests/org/jetbrains/jet/completion/MultiFileJvmBasicCompletionTestGenerated.java @@ -36,6 +36,11 @@ public class MultiFileJvmBasicCompletionTestGenerated extends AbstractMultiFileJ JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("idea/testData/completion/basic/multifile"), Pattern.compile("^([^\\.]+)$"), false); } + @TestMetadata("CompleteFunctionWithNoSpecifiedType") + public void testCompleteFunctionWithNoSpecifiedType() throws Exception { + doTest("idea/testData/completion/basic/multifile/CompleteFunctionWithNoSpecifiedType"); + } + @TestMetadata("CompleteImportedFunction") public void testCompleteImportedFunction() throws Exception { doTest("idea/testData/completion/basic/multifile/CompleteImportedFunction"); diff --git a/idea/tests/org/jetbrains/jet/plugin/stubs/AbstractLazyResolveByStubTest.java b/idea/tests/org/jetbrains/jet/plugin/stubs/AbstractLazyResolveByStubTest.java index 59bcd8dbfc7..ddb00382bf2 100644 --- a/idea/tests/org/jetbrains/jet/plugin/stubs/AbstractLazyResolveByStubTest.java +++ b/idea/tests/org/jetbrains/jet/plugin/stubs/AbstractLazyResolveByStubTest.java @@ -24,6 +24,8 @@ import com.intellij.openapi.roots.ModifiableRootModel; import com.intellij.openapi.util.io.FileUtil; import com.intellij.testFramework.LightProjectDescriptor; import com.intellij.util.Consumer; +import kotlin.Function0; +import kotlin.Unit; import org.jetbrains.annotations.NotNull; import org.jetbrains.jet.lang.descriptors.ModuleDescriptor; import org.jetbrains.jet.lang.descriptors.PackageViewDescriptor; @@ -32,7 +34,6 @@ import org.jetbrains.jet.lang.resolve.name.FqName; import org.jetbrains.jet.lang.types.lang.KotlinBuiltIns; import org.jetbrains.jet.plugin.JetWithJdkAndRuntimeLightProjectDescriptor; import org.jetbrains.jet.plugin.caches.resolve.KotlinCacheService; -import org.jetbrains.jet.plugin.project.AnalyzerFacadeWithCache; import org.jetbrains.jet.plugin.project.ResolveSessionForBodies; import org.jetbrains.jet.test.util.RecursiveDescriptorComparator; import org.junit.Assert; @@ -52,11 +53,26 @@ public abstract class AbstractLazyResolveByStubTest extends CodeInsightTestCase doTest(testFileName, false, false); } - public void doTest(@NotNull String path, boolean checkPrimaryConstructors, boolean checkPropertyAccessors) throws Exception { + public void doTest(@NotNull final String path, final boolean checkPrimaryConstructors, final boolean checkPropertyAccessors) + throws Exception { configureByFile(path); - AstAccessControl.instance$.prohibitAstAccessForKotlinFiles(getProject(), getTestRootDisposable()); configureModule(getModule(), JetWithJdkAndRuntimeLightProjectDescriptor.INSTANCE); - ResolveSessionForBodies resolveSession = KotlinCacheService.object$.getInstance(getFile().getProject()).getLazyResolveSession((JetFile) getFile()); + boolean shouldFail = getTestName(false).equals("ClassWithConstVal"); + AstAccessControl.instance$.testWithControlledAccessToAst( + shouldFail, getProject(), getTestRootDisposable(), + new Function0() { + @Override + public Unit invoke() { + performTest(path, checkPrimaryConstructors, checkPropertyAccessors); + return Unit.VALUE; + } + } + ); + } + + private void performTest(@NotNull String path, boolean checkPrimaryConstructors, boolean checkPropertyAccessors) { + ResolveSessionForBodies resolveSession = + KotlinCacheService.object$.getInstance(getFile().getProject()).getLazyResolveSession((JetFile) getFile()); ModuleDescriptor module = resolveSession.getModuleDescriptor(); PackageViewDescriptor packageViewDescriptor = module.getPackage(new FqName("test")); Assert.assertNotNull(packageViewDescriptor); diff --git a/idea/tests/org/jetbrains/jet/plugin/stubs/AstAccessControl.kt b/idea/tests/org/jetbrains/jet/plugin/stubs/AstAccessControl.kt index 7a282581fac..55d6f8155e0 100644 --- a/idea/tests/org/jetbrains/jet/plugin/stubs/AstAccessControl.kt +++ b/idea/tests/org/jetbrains/jet/plugin/stubs/AstAccessControl.kt @@ -27,16 +27,39 @@ import com.intellij.openapi.util.io.FileUtil import com.intellij.openapi.vfs.VfsUtil import com.intellij.openapi.vfs.VfsUtilCore import org.jetbrains.jet.InTextDirectivesUtils +import junit.framework.TestCase +import kotlin.test.fail object AstAccessControl { + private val ALLOW_AST_ACCESS_DIRECTIVE = "ALLOW_AST_ACCESS" - val ALLOW_AST_ACCESS_DIRECTIVE = "ALLOW_AST_ACCESS" + // Please provide at least one test that fails ast switch check (shouldFail should be true for at least one test) + // This kind of inconvenience is justified by the fact that the check can be invalidated by slight misconfiguration of the test + // leading to all tests passing + fun testWithControlledAccessToAst(shouldFail: Boolean, project: Project, disposable: Disposable, testBody: () -> Unit) { + testWithControlledAccessToAst(shouldFail, listOf(), project, disposable, testBody) + } - fun prohibitAstAccessForKotlinFiles(project: Project, disposable: Disposable) { + fun testWithControlledAccessToAst( + shouldFail: Boolean, allowedFile: VirtualFile, + project: Project, disposable: Disposable, testBody: () -> Unit + ) { + testWithControlledAccessToAst(shouldFail, listOf(allowedFile), project, disposable, testBody) + } + + fun testWithControlledAccessToAst( + shouldFail: Boolean, allowedFiles: List, + project: Project, disposable: Disposable, testBody: () -> Unit + ) { + setFilter(allowedFiles, disposable, project) + performTest(shouldFail, testBody) + } + + private fun setFilter(allowedFiles: List, disposable: Disposable, project: Project) { val manager = (PsiManager.getInstance(project) as PsiManagerImpl) val filter = VirtualFileFilter { file -> - if (file!!.getFileType() != JetFileType.INSTANCE) { + if (file!!.getFileType() != JetFileType.INSTANCE || file in allowedFiles) { false } else { @@ -47,4 +70,20 @@ object AstAccessControl { manager.setAssertOnFileLoadingFilter(filter, disposable) } -} \ No newline at end of file + private fun performTest(shouldFail: Boolean, testBody: () -> Unit) { + try { + testBody() + if (shouldFail) { + fail("This failure means that that a test that should fail (by triggering ast switch) in fact did not.\n" + + "This could happen for the following reasons:\n" + + "1. This kind of operation no longer trigger ast switch, choose better indicator test case." + + "2. Test is now misconfigured and no longer checks for ast switch, reconfigure the test.") + } + } + catch (e: Throwable) { + if (!shouldFail) { + throw e + } + } + } +}