Optimize the inspectors and enable them to fail silently for expected…

… exception types (#3121) * Optimize the inspectors and enable them to fail silently for expected exception types * Update sqldelight-idea-plugin/src/main/kotlin/app/cash/sqldelight/intellij/inspections/InspectionExt.kt Co-authored-by: Jake Wharton <github@jakewharton.com> * Fix up failing tests Co-authored-by: Jake Wharton <github@jakewharton.com>
cashapp · Apr 25, 2022 · f9edfe0 · f9edfe0
1 parent 7036ffe
commit f9edfe0
Show file tree

Hide file tree

Showing 20 changed files with 168 additions and 154 deletions.
diff --git a/sqldelight-compiler/src/main/kotlin/app/cash/sqldelight/core/compiler/SqlDelightCompiler.kt b/sqldelight-compiler/src/main/kotlin/app/cash/sqldelight/core/compiler/SqlDelightCompiler.kt
@@ -44,9 +44,13 @@ object SqlDelightCompiler {
     file: SqlDelightQueriesFile,
     output: FileAppender
   ) {
-    writeTableInterfaces(file, output)
-    writeQueryInterfaces(file, output)
-    writeQueries(module, dialect, file, output)
+    try {
+      writeTableInterfaces(file, output)
+      writeQueryInterfaces(file, output)
+      writeQueries(module, dialect, file, output)
+    } catch (e: InvalidElementDetectedException) {
+      // It's okay if compilation is cut short, we can just quit out.
+    }
   }
 
   fun writeInterfaces(

diff --git a/sqldelight-compiler/src/main/kotlin/app/cash/sqldelight/core/compiler/model/NamedQuery.kt b/sqldelight-compiler/src/main/kotlin/app/cash/sqldelight/core/compiler/model/NamedQuery.kt
@@ -56,7 +56,7 @@ data class NamedQuery(
    * Explodes the sqlite query into an ordered list (same order as the query) of types to be exposed
    * by the generated api.
    */
-  internal val resultColumns: List<IntermediateType> by lazy {
+  val resultColumns: List<IntermediateType> by lazy {
     if (queryable is SelectQueryable) resultColumns(queryable.select)
     else queryable.select.typesExposed(LinkedHashSet())
   }

diff --git a/sqldelight-compiler/src/main/kotlin/app/cash/sqldelight/core/lang/SqlDelightQueriesFile.kt b/sqldelight-compiler/src/main/kotlin/app/cash/sqldelight/core/lang/SqlDelightQueriesFile.kt
@@ -44,7 +44,7 @@ class SqlDelightQueriesFile(
     }
   }
 
-  internal val namedQueries by lazy {
+  val namedQueries by lazy {
     transactions().filterIsInstance<NamedQuery>() + sqlStatements()
       .filter { typeResolver.queryWithResults(it.statement) != null && it.identifier.name != null }
       .map {

diff --git a/sqldelight-compiler/src/main/kotlin/app/cash/sqldelight/core/lang/util/TreeUtil.kt b/sqldelight-compiler/src/main/kotlin/app/cash/sqldelight/core/lang/util/TreeUtil.kt
@@ -41,7 +41,6 @@ import com.alecstrong.sql.psi.core.psi.SqlTypeName
 import com.alecstrong.sql.psi.core.psi.SqlTypes
 import com.alecstrong.sql.psi.core.psi.mixins.ColumnDefMixin
 import com.intellij.lang.ASTNode
-import com.intellij.openapi.progress.ProcessCanceledException
 import com.intellij.psi.PsiDirectory
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiWhiteSpace
@@ -57,7 +56,7 @@ internal inline fun <reified R : PsiElement> PsiElement.parentOfType(): R {
   return PsiTreeUtil.getParentOfType(this, R::class.java)!!
 }
 
-internal fun PsiElement.type(): IntermediateType = if (!isValid) throw ProcessCanceledException() else when (this) {
+internal fun PsiElement.type(): IntermediateType = when (this) {
   is ExposableType -> type()
   is SqlTypeName -> sqFile().typeResolver.definitionType(this)
   is AliasElement -> source().type().copy(name = name)

diff --git a/...ght-idea-plugin/src/main/kotlin/app/cash/sqldelight/intellij/inspections/InspectionExt.kt b/...ght-idea-plugin/src/main/kotlin/app/cash/sqldelight/intellij/inspections/InspectionExt.kt
@@ -0,0 +1,53 @@
+package app.cash.sqldelight.intellij.inspections
+
+import app.cash.sqldelight.core.SqlDelightFileIndex
+import app.cash.sqldelight.core.lang.SqlDelightFile
+import com.alecstrong.sql.psi.core.psi.InvalidElementDetectedException
+import com.intellij.codeInspection.LocalInspectionTool
+import com.intellij.codeInspection.ProblemDescriptor
+import com.intellij.openapi.module.Module
+import com.intellij.psi.PsiElementVisitor
+import com.intellij.psi.PsiFile
+import com.intellij.psi.PsiInvalidElementAccessException
+
+internal inline fun LocalInspectionTool.ensureReady(file: PsiFile, block: InspectionProperties.() -> PsiElementVisitor): PsiElementVisitor {
+  return ensureReady(file, { PsiElementVisitor.EMPTY_VISITOR }, block)
+}
+
+internal inline fun LocalInspectionTool.ensureFileReady(file: PsiFile, block: InspectionProperties.() -> Array<ProblemDescriptor>): Array<ProblemDescriptor> {
+  return ensureReady(file, { emptyArray() }, block)
+}
+
+@Suppress("unused") // Receiver to enforce usage.
+internal inline fun PsiElementVisitor.ignoreInvalidElements(block: () -> Unit) {
+  try {
+    block()
+  } catch (_: InvalidElementDetectedException) {
+  } catch (_: PsiInvalidElementAccessException) {
+  }
+}
+
+private inline fun <T> ensureReady(
+  file: PsiFile,
+  defaultValue: () -> T,
+  block: InspectionProperties.() -> T
+): T {
+  val sqlDelightFile = file as? SqlDelightFile ?: return defaultValue()
+  val module = file.module ?: return defaultValue()
+  val fileIndex = SqlDelightFileIndex.getInstance(module)
+  if (!fileIndex.isConfigured) return defaultValue()
+
+  try {
+    return InspectionProperties(module, sqlDelightFile, fileIndex).block()
+  } catch (_: InvalidElementDetectedException) {
+    return defaultValue()
+  } catch (_: PsiInvalidElementAccessException) {
+    return defaultValue()
+  }
+}
+
+internal data class InspectionProperties(
+  val module: Module,
+  val sqlDelightFile: SqlDelightFile,
+  val fileIndex: SqlDelightFileIndex,
+)
diff --git a/.../src/main/kotlin/app/cash/sqldelight/intellij/inspections/MismatchJoinColumnInspection.kt b/.../src/main/kotlin/app/cash/sqldelight/intellij/inspections/MismatchJoinColumnInspection.kt
@@ -1,7 +1,5 @@
 package app.cash.sqldelight.intellij.inspections
 
-import app.cash.sqldelight.core.SqlDelightProjectService
-import app.cash.sqldelight.core.lang.SqlDelightFile
 import app.cash.sqldelight.core.lang.psi.isColumnSameAs
 import app.cash.sqldelight.core.lang.psi.isTypeSameAs
 import app.cash.sqldelight.core.lang.util.findChildrenOfType
@@ -12,7 +10,6 @@ import com.alecstrong.sql.psi.core.psi.SqlJoinConstraint
 import com.alecstrong.sql.psi.core.psi.SqlParenExpr
 import com.intellij.codeInspection.InspectionManager
 import com.intellij.codeInspection.LocalInspectionTool
-import com.intellij.codeInspection.ProblemDescriptor
 import com.intellij.codeInspection.ProblemHighlightType
 import com.intellij.psi.PsiFile
 
@@ -21,15 +18,7 @@ internal class MismatchJoinColumnInspection : LocalInspectionTool() {
     file: PsiFile,
     manager: InspectionManager,
     isOnTheFly: Boolean
-  ): Array<ProblemDescriptor> {
-    if (file !is SqlDelightFile) return emptyArray()
-
-    val projectService = SqlDelightProjectService.getInstance(file.project)
-    if (file.module?.let { module -> projectService.fileIndex(module).isConfigured } != true) {
-      // Do not attempt to inspect the file types if the project is not configured yet.
-      return emptyArray()
-    }
-
+  ) = ensureFileReady(file) {
     val joinConstraints = file.findChildrenOfType<SqlJoinConstraint>()
       .mapNotNull { it.expr?.binaryEqualityExpr() }
 

diff --git a/...otlin/app/cash/sqldelight/intellij/inspections/MixedNamedAndPositionalParamsInspection.kt b/...otlin/app/cash/sqldelight/intellij/inspections/MixedNamedAndPositionalParamsInspection.kt
@@ -10,16 +10,15 @@ import com.alecstrong.sql.psi.core.psi.SqlVisitor
 import com.intellij.codeInspection.LocalInspectionTool
 import com.intellij.codeInspection.ProblemHighlightType
 import com.intellij.codeInspection.ProblemsHolder
-import com.intellij.psi.PsiElementVisitor
 
 private val positional = "^\\?\\d*\$".toRegex()
 private val named = "^[:@$][a-zA-Z0-9]*\$".toRegex()
 
 internal class MixedNamedAndPositionalParamsInspection : LocalInspectionTool() {
 
-  override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
+  override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean) = ensureReady(holder.file) {
     return object : SqlVisitor() {
-      override fun visitExpr(o: SqlExpr) {
+      override fun visitExpr(o: SqlExpr) = ignoreInvalidElements {
         if (o !is SqlBinaryExpr) return
 
         checkExpression(o)

diff --git a/...plugin/src/main/kotlin/app/cash/sqldelight/intellij/inspections/NullEqualityInspection.kt b/...plugin/src/main/kotlin/app/cash/sqldelight/intellij/inspections/NullEqualityInspection.kt
@@ -8,18 +8,17 @@ import com.intellij.codeInspection.LocalInspectionTool
 import com.intellij.codeInspection.LocalInspectionToolSession
 import com.intellij.codeInspection.ProblemHighlightType
 import com.intellij.codeInspection.ProblemsHolder
-import com.intellij.psi.PsiElementVisitor
 
 internal class NullEqualityInspection : LocalInspectionTool() {
 
   override fun buildVisitor(
     holder: ProblemsHolder,
     isOnTheFly: Boolean,
     session: LocalInspectionToolSession
-  ): PsiElementVisitor {
+  ) = ensureReady(session.file) {
     return object : SqlVisitor() {
 
-      override fun visitBinaryEqualityExpr(o: SqlBinaryEqualityExpr) {
+      override fun visitBinaryEqualityExpr(o: SqlBinaryEqualityExpr) = ignoreInvalidElements {
         val exprList = o.getExprList()
         if (exprList.size < 2) return
         val (expr1, expr2) = exprList

diff --git a/...lugin/src/main/kotlin/app/cash/sqldelight/intellij/inspections/OptimisticLockAnnotator.kt b/...lugin/src/main/kotlin/app/cash/sqldelight/intellij/inspections/OptimisticLockAnnotator.kt
@@ -7,7 +7,7 @@ import com.intellij.codeInsight.intention.IntentionAction
 import com.intellij.psi.PsiElement
 
 class OptimisticLockAnnotator : OptimisticLockValidator() {
-  override fun quickFix(element: PsiElement, lock: ColumnDefMixin): IntentionAction? {
+  override fun quickFix(element: PsiElement, lock: ColumnDefMixin): IntentionAction {
     return AddOptimisticLockIntention(element, lock)
   }
 }
diff --git a/.../src/main/kotlin/app/cash/sqldelight/intellij/inspections/RedundantNullCheckInspection.kt b/.../src/main/kotlin/app/cash/sqldelight/intellij/inspections/RedundantNullCheckInspection.kt
@@ -10,7 +10,6 @@ import com.intellij.codeInspection.LocalInspectionTool
 import com.intellij.codeInspection.LocalInspectionToolSession
 import com.intellij.codeInspection.ProblemHighlightType
 import com.intellij.codeInspection.ProblemsHolder
-import com.intellij.psi.PsiElementVisitor
 import com.intellij.psi.util.PsiTreeUtil
 import com.intellij.psi.util.elementType
 import com.intellij.psi.util.parentOfType
@@ -21,27 +20,29 @@ internal class RedundantNullCheckInspection : LocalInspectionTool() {
     holder: ProblemsHolder,
     isOnTheFly: Boolean,
     session: LocalInspectionToolSession
-  ): PsiElementVisitor = object : SqlVisitor() {
+  ) = ensureReady(session.file) {
+    object : SqlVisitor() {
 
-    override fun visitIsExpr(o: SqlIsExpr) {
-      if (o.context !is SqlSelectStmt) return
-      if (PsiTreeUtil.prevVisibleLeaf(o).elementType != SqlTypes.WHERE) return
+      override fun visitIsExpr(o: SqlIsExpr) = ignoreInvalidElements {
+        if (o.context !is SqlSelectStmt) return
+        if (PsiTreeUtil.prevVisibleLeaf(o).elementType != SqlTypes.WHERE) return
 
-      val firstChild = o.firstChild
-      if (firstChild !is SqlColumnExpr) return
-      val clauseText = o.text
-      if (!clauseText.endsWith("IS NOT NULL") && !clauseText.endsWith("IS NULL")) return
+        val firstChild = o.firstChild
+        if (firstChild !is SqlColumnExpr) return
+        val clauseText = o.text
+        if (!clauseText.endsWith("IS NOT NULL") && !clauseText.endsWith("IS NULL")) return
 
-      val columnName = firstChild.columnName
-      val sqlColumnDef = columnName.reference?.resolve()?.parentOfType<SqlColumnDef>() ?: return
-      val hasNotNullConstraint = sqlColumnDef.columnConstraintList.any { constraint ->
-        constraint.textMatches("NOT NULL")
-      }
-      if (!hasNotNullConstraint) {
-        return
-      }
+        val columnName = firstChild.columnName
+        val sqlColumnDef = columnName.reference?.resolve()?.parentOfType<SqlColumnDef>() ?: return
+        val hasNotNullConstraint = sqlColumnDef.columnConstraintList.any { constraint ->
+          constraint.textMatches("NOT NULL")
+        }
+        if (!hasNotNullConstraint) {
+          return
+        }
 
-      holder.registerProblem(o, "Column ${columnName.name} defined as NOT NULL", ProblemHighlightType.WARNING)
+        holder.registerProblem(o, "Column ${columnName.name} defined as NOT NULL", ProblemHighlightType.WARNING)
+      }
     }
   }
 }
diff --git a/...rc/main/kotlin/app/cash/sqldelight/intellij/inspections/SchemaNeedsMigrationInspection.kt b/...rc/main/kotlin/app/cash/sqldelight/intellij/inspections/SchemaNeedsMigrationInspection.kt
@@ -1,9 +1,7 @@
 package app.cash.sqldelight.intellij.inspections
 
-import app.cash.sqldelight.core.SqlDelightFileIndex
 import app.cash.sqldelight.core.SqlDelightProjectService
 import app.cash.sqldelight.core.lang.MigrationFile
-import app.cash.sqldelight.core.lang.SqlDelightQueriesFile
 import app.cash.sqldelight.core.lang.psi.parameterValue
 import app.cash.sqldelight.core.lang.util.migrationFiles
 import app.cash.sqldelight.intellij.refactoring.SqlDelightSignatureBuilder
@@ -17,7 +15,6 @@ import com.intellij.codeInspection.ProblemHighlightType.GENERIC_ERROR
 import com.intellij.codeInspection.ProblemsHolder
 import com.intellij.openapi.project.Project
 import com.intellij.psi.PsiElement
-import com.intellij.psi.PsiElementVisitor
 import com.intellij.psi.PsiFile
 import com.intellij.psi.SmartPointerManager
 import com.intellij.refactoring.suggested.SuggestedRefactoringSupport.Signature
@@ -29,22 +26,21 @@ internal class SchemaNeedsMigrationInspection : LocalInspectionTool() {
   override fun buildVisitor(
     holder: ProblemsHolder,
     isOnTheFly: Boolean
-  ): PsiElementVisitor = object : SqlVisitor() {
-    override fun visitCreateTableStmt(createTable: SqlCreateTableStmt) {
-      val file = createTable.containingFile as? SqlDelightQueriesFile ?: return
-      val module = file.module ?: return
-
-      val dbFile = file.findDbFile() ?: return
-      val fileIndex = SqlDelightFileIndex.getInstance(module)
-      val topMigrationFile = fileIndex.sourceFolders(file).asSequence()
-        .flatMap { it.migrationFiles() }
-        .maxByOrNull { it.version }
-
-      val tables = (topMigrationFile ?: dbFile).tables(true)
-      val tableWithSameName = tables.find { it.tableName.name == createTable.tableName.name }
-      val signature = signatureBuilder.signature(createTable) ?: return
-
-      if (tableWithSameName == null) {
+  ) = ensureReady(holder.file) {
+    object : SqlVisitor() {
+      override fun visitCreateTableStmt(createTable: SqlCreateTableStmt) = ignoreInvalidElements {
+        val dbFile = sqlDelightFile.findDbFile() ?: return
+        val topMigrationFile = fileIndex.sourceFolders(sqlDelightFile).asSequence()
+          .flatMap { it.migrationFiles() }
+          .maxByOrNull { it.version }
+
+        val tables = (topMigrationFile ?: dbFile).tables(true)
+        val tableWithSameName = tables.find { it.tableName.name == createTable.tableName.name }
+        val signature = signatureBuilder.signature(createTable) ?: return
+
+        if (tableWithSameName == null) {
+        /*
+         * TODO: Reenable this once it performs faster. Evaluating oldQuery.query is expensive.
         val tableWithDifferentName = tables.find { oldQuery ->
           val oldColumns = oldQuery.query.columns
           oldColumns.size == signature.parameters.size &&
@@ -59,27 +55,28 @@ internal class SchemaNeedsMigrationInspection : LocalInspectionTool() {
             )
           )
           return
-        }
+        }*/
 
-        holder.registerProblem(
-          createTable.tableName, "Table needs to be added in a migration", GENERIC_ERROR,
-          CreateTableMigrationQuickFix(createTable, topMigrationFile)
-        )
-        return
-      }
+          holder.registerProblem(
+            createTable.tableName, "Table needs to be added in a migration", GENERIC_ERROR,
+            CreateTableMigrationQuickFix(createTable, topMigrationFile)
+          )
+          return
+        }
 
-      val oldSignature = Signature.create(
-        name = tableWithSameName.tableName.name,
-        type = null,
-        parameters = tableWithSameName.query.columns.mapNotNull { it.parameterValue() },
-        additionalData = null,
-      ) ?: return
+        val oldSignature = Signature.create(
+          name = tableWithSameName.tableName.name,
+          type = null,
+          parameters = tableWithSameName.query.columns.mapNotNull { it.parameterValue() },
+          additionalData = null,
+        ) ?: return
 
-      if (oldSignature != signature) {
-        holder.registerProblem(
-          createTable.tableName, "Table needs to be altered in a migration", GENERIC_ERROR,
-          AlterTableMigrationQuickFix(createTable, oldSignature, signature)
-        )
+        if (oldSignature != signature) {
+          holder.registerProblem(
+            createTable.tableName, "Table needs to be altered in a migration", GENERIC_ERROR,
+            AlterTableMigrationQuickFix(createTable, oldSignature, signature)
+          )
+        }
       }
     }
   }