Report an error on unclosed comments (#520)

Summary: Otherwise formatting succeeds and moves all subsequent code into the comment too. This is especially bad for nested comments, which have a surprising syntax. Pull Request resolved: #520 Reviewed By: cortinico Differential Revision: D65667051 Pulled By: hick209 fbshipit-source-id: 5da1c66049fd99c72a0fcf5ec5c55232c2bc3667
facebook · Nov 8, 2024 · 32cdc10 · 32cdc10
1 parent 3f10f99
commit 32cdc10
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 0 deletions.
diff --git a/core/src/main/java/com/facebook/ktfmt/format/Tokenizer.kt b/core/src/main/java/com/facebook/ktfmt/format/Tokenizer.kt
@@ -17,6 +17,7 @@
 package com.facebook.ktfmt.format
 
 import java.util.regex.Pattern
+import org.jetbrains.kotlin.com.intellij.openapi.util.text.StringUtil
 import org.jetbrains.kotlin.com.intellij.psi.PsiComment
 import org.jetbrains.kotlin.com.intellij.psi.PsiElement
 import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace
@@ -53,6 +54,10 @@ class Tokenizer(private val fileText: String, val file: KtFile) : KtTreeVisitorV
     val originalText = fileText.substring(startIndex, endIndex)
     when (element) {
       is PsiComment -> {
+        if (element.text.startsWith("/*") && !element.text.endsWith("*/")) {
+          throw ParseError(
+              "Unclosed comment", StringUtil.offsetToLineColumn(fileText, element.startOffset))
+        }
         toks.add(
             KotlinTok(
                 index = index,

diff --git a/core/src/test/java/com/facebook/ktfmt/format/TokenizerTest.kt b/core/src/test/java/com/facebook/ktfmt/format/TokenizerTest.kt
@@ -17,6 +17,7 @@
 package com.facebook.ktfmt.format
 
 import com.google.common.truth.Truth.assertThat
+import kotlin.test.assertFailsWith
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
@@ -218,4 +219,63 @@ class TokenizerTest {
             23)
         .inOrder()
   }
+
+  @Test
+  fun `Unclosed comment obvious`() {
+    assertParseError(
+        """
+      |package a.b
+      |/*
+      |class A {}
+      |"""
+            .trimMargin(),
+        "2:1: error: Unclosed comment")
+  }
+
+  @Test
+  fun `Unclosed comment too short`() {
+    assertParseError(
+        """
+      |package a.b
+      |/*/
+      |class A {}
+      |"""
+            .trimMargin(),
+        "2:1: error: Unclosed comment")
+  }
+
+  @Test
+  fun `Unclosed comment nested`() {
+    assertParseError(
+        """
+      |package a.b
+      |/* /* */
+      |class A {}
+      |"""
+            .trimMargin(),
+        "2:1: error: Unclosed comment")
+  }
+
+  @Test
+  fun `Unclosed comment nested EOF`() {
+    // TODO: https://youtrack.jetbrains.com/issue/KT-72887 - This should be an error.
+    assertParseError(
+        """
+      |package a.b
+      |class A {}
+      |/* /* */"""
+            .trimMargin(),
+        null)
+  }
+
+  private fun assertParseError(code: String, message: String?) {
+    val file = Parser.parse(code)
+    val tokenizer = Tokenizer(code, file)
+    if (message == null) {
+      file.accept(tokenizer)
+    } else {
+      val e = assertFailsWith<ParseError> { file.accept(tokenizer) }
+      assertThat(e).hasMessageThat().isEqualTo(message)
+    }
+  }
 }