From ba948cda38eef6152967bb3f32cbf6ee2e1d65a1 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Tue, 30 Jun 2020 17:17:37 +0200 Subject: [PATCH] Report warning on characters which can cause problems on Windows As soon as we fix KT-17438, this warning will be turned into an error. --- ...irOldFrontendDiagnosticsTestGenerated.java | 5 +++ .../checkers/JvmSimpleNameBacktickChecker.kt | 28 +++++++++---- .../diagnostics/DefaultErrorMessagesJvm.java | 2 + .../resolve/jvm/diagnostics/ErrorsJvm.java | 2 + .../nameWithDangerousCharacters.fir.kt | 12 ++++++ .../nameWithDangerousCharacters.kt | 12 ++++++ .../nameWithDangerousCharacters.txt | 39 +++++++++++++++++++ .../checkers/DiagnosticsTestGenerated.java | 5 +++ .../DiagnosticsUsingJavacTestGenerated.java | 5 +++ 9 files changed, 102 insertions(+), 8 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/declarationChecks/nameWithDangerousCharacters.fir.kt create mode 100644 compiler/testData/diagnostics/tests/declarationChecks/nameWithDangerousCharacters.kt create mode 100644 compiler/testData/diagnostics/tests/declarationChecks/nameWithDangerousCharacters.txt diff --git a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirOldFrontendDiagnosticsTestGenerated.java index 2058393a3cf..7ebf99e32c0 100644 --- a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirOldFrontendDiagnosticsTestGenerated.java @@ -5616,6 +5616,11 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirOldFronte runTest("compiler/testData/diagnostics/tests/declarationChecks/MultiDeclarationErrors.kt"); } + @TestMetadata("nameWithDangerousCharacters.kt") + public void testNameWithDangerousCharacters() throws Exception { + runTest("compiler/testData/diagnostics/tests/declarationChecks/nameWithDangerousCharacters.kt"); + } + @TestMetadata("namedFunAsLastExpressionInBlock.kt") public void testNamedFunAsLastExpressionInBlock() throws Exception { runTest("compiler/testData/diagnostics/tests/declarationChecks/namedFunAsLastExpressionInBlock.kt"); diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JvmSimpleNameBacktickChecker.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JvmSimpleNameBacktickChecker.kt index e8c5969f883..c7f35b7653a 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JvmSimpleNameBacktickChecker.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JvmSimpleNameBacktickChecker.kt @@ -21,11 +21,15 @@ import org.jetbrains.kotlin.diagnostics.DiagnosticSink import org.jetbrains.kotlin.diagnostics.Errors import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.resolve.IdentifierChecker +import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm object JvmSimpleNameBacktickChecker : IdentifierChecker { // See The Java Virtual Machine Specification, section 4.7.9.1 https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.9.1 val INVALID_CHARS = setOf('.', ';', '[', ']', '/', '<', '>', ':', '\\') + // These characters can cause problems on Windows. '?*"|' are not allowed in file names, and % leads to unexpected env var expansion. + private val DANGEROUS_CHARS = setOf('?', '*', '"', '|', '%') + override fun checkIdentifier(simpleNameExpression: KtSimpleNameExpression, diagnosticHolder: DiagnosticSink) { reportIfNeeded(simpleNameExpression.getReferencedName(), { simpleNameExpression.getIdentifier() }, diagnosticHolder) } @@ -53,15 +57,23 @@ object JvmSimpleNameBacktickChecker : IdentifierChecker { private fun reportIfNeeded(name: String, reportOn: () -> PsiElement?, diagnosticHolder: DiagnosticSink) { val text = KtPsiUtil.unquoteIdentifier(name) - if (text.isEmpty()) { - diagnosticHolder.report(Errors.INVALID_CHARACTERS.on(reportOn() ?: return, "should not be empty")) - } else if (text.any { it in INVALID_CHARS }) { - diagnosticHolder.report( - Errors.INVALID_CHARACTERS.on( - reportOn() ?: return, - "contains illegal characters: ${INVALID_CHARS.intersect(text.toSet()).joinToString("")}" + when { + text.isEmpty() -> { + diagnosticHolder.report(Errors.INVALID_CHARACTERS.on(reportOn() ?: return, "should not be empty")) + } + text.any { it in INVALID_CHARS } -> { + diagnosticHolder.report( + Errors.INVALID_CHARACTERS.on( + reportOn() ?: return, + "contains illegal characters: ${INVALID_CHARS.intersect(text.toSet()).joinToString("")}" + ) ) - ) + } + text.any { it in DANGEROUS_CHARS } -> { + diagnosticHolder.report( + ErrorsJvm.DANGEROUS_CHARACTERS.on(reportOn() ?: return, DANGEROUS_CHARS.intersect(text.toSet()).joinToString("")) + ) + } } } } diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java index 7041fc961be..2608142f60c 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java @@ -165,6 +165,8 @@ public class DefaultErrorMessagesJvm implements DefaultErrorMessages.Extension { "or annotate the class with @JvmDefaultWithoutCompatibility. " + "Please refer to KT-39603 for details", COMPACT, SHORT_NAMES_IN_TYPES); + + MAP.put(DANGEROUS_CHARACTERS, "Name contains characters which can cause problems on Windows: {0}", STRING); } @NotNull diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java index 2cac1e51c0b..6ebcdd8c0f1 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java @@ -141,6 +141,8 @@ public interface ErrorsJvm { DiagnosticFactory2 EXPLICIT_OVERRIDE_REQUIRED_IN_COMPATIBILITY_MODE = DiagnosticFactory2.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT); + DiagnosticFactory1 DANGEROUS_CHARACTERS = DiagnosticFactory1.create(WARNING); + @SuppressWarnings("UnusedDeclaration") Object _initializer = new Object() { { diff --git a/compiler/testData/diagnostics/tests/declarationChecks/nameWithDangerousCharacters.fir.kt b/compiler/testData/diagnostics/tests/declarationChecks/nameWithDangerousCharacters.fir.kt new file mode 100644 index 00000000000..28615d6a40a --- /dev/null +++ b/compiler/testData/diagnostics/tests/declarationChecks/nameWithDangerousCharacters.fir.kt @@ -0,0 +1,12 @@ +class `A?B` +class `A*B` +class `A"B` +class `A|B` +class `A%B` + +fun `?*"|%`(): Int { + val `?` = 0 + return `?` +} + +val `"a"+"b"` = "c" diff --git a/compiler/testData/diagnostics/tests/declarationChecks/nameWithDangerousCharacters.kt b/compiler/testData/diagnostics/tests/declarationChecks/nameWithDangerousCharacters.kt new file mode 100644 index 00000000000..44860a1c6a1 --- /dev/null +++ b/compiler/testData/diagnostics/tests/declarationChecks/nameWithDangerousCharacters.kt @@ -0,0 +1,12 @@ +class `A?B` +class `A*B` +class `A"B` +class `A|B` +class `A%B` + +fun `?*"|%`(): Int { + val `?` = 0 + return `?` +} + +val `"a"+"b"` = "c" diff --git a/compiler/testData/diagnostics/tests/declarationChecks/nameWithDangerousCharacters.txt b/compiler/testData/diagnostics/tests/declarationChecks/nameWithDangerousCharacters.txt new file mode 100644 index 00000000000..c4ae7afc98f --- /dev/null +++ b/compiler/testData/diagnostics/tests/declarationChecks/nameWithDangerousCharacters.txt @@ -0,0 +1,39 @@ +package + +public val `"a"+"b"`: kotlin.String = "c" +public fun `?*"|%`(): kotlin.Int + +public final class `A"B` { + public constructor `A"B`() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class `A%B` { + public constructor `A%B`() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class `A*B` { + public constructor `A*B`() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class `A?B` { + public constructor `A?B`() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class `A|B` { + public constructor `A|B`() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index f020c7ffa5b..6ac10915d26 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -5623,6 +5623,11 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTestWithFirVali runTest("compiler/testData/diagnostics/tests/declarationChecks/MultiDeclarationErrors.kt"); } + @TestMetadata("nameWithDangerousCharacters.kt") + public void testNameWithDangerousCharacters() throws Exception { + runTest("compiler/testData/diagnostics/tests/declarationChecks/nameWithDangerousCharacters.kt"); + } + @TestMetadata("namedFunAsLastExpressionInBlock.kt") public void testNamedFunAsLastExpressionInBlock() throws Exception { runTest("compiler/testData/diagnostics/tests/declarationChecks/namedFunAsLastExpressionInBlock.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java index f54e8945748..7d56bba5ef8 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java @@ -5618,6 +5618,11 @@ public class DiagnosticsUsingJavacTestGenerated extends AbstractDiagnosticsUsing runTest("compiler/testData/diagnostics/tests/declarationChecks/MultiDeclarationErrors.kt"); } + @TestMetadata("nameWithDangerousCharacters.kt") + public void testNameWithDangerousCharacters() throws Exception { + runTest("compiler/testData/diagnostics/tests/declarationChecks/nameWithDangerousCharacters.kt"); + } + @TestMetadata("namedFunAsLastExpressionInBlock.kt") public void testNamedFunAsLastExpressionInBlock() throws Exception { runTest("compiler/testData/diagnostics/tests/declarationChecks/namedFunAsLastExpressionInBlock.kt");