From 9ba06a90d2790ab0d282efb947ca479874f5a849 Mon Sep 17 00:00:00 2001 From: Ilya Zorin Date: Mon, 18 Dec 2017 16:52:13 +0200 Subject: [PATCH] Question marks in a query don't match number of arguments lint check (#220) --- build.gradle | 7 + settings.gradle | 1 + sqlbrite-lint/build.gradle | 16 ++ .../squareup/sqlbrite3/BriteIssueRegistry.kt | 23 ++ .../sqlbrite3/SqlBriteArgCountDetector.kt | 79 +++++++ .../sqlbrite3/SqlBriteArgCountDetectorTest.kt | 207 ++++++++++++++++++ sqlbrite/build.gradle | 2 + 7 files changed, 335 insertions(+) create mode 100644 sqlbrite-lint/build.gradle create mode 100644 sqlbrite-lint/src/main/java/com/squareup/sqlbrite3/BriteIssueRegistry.kt create mode 100644 sqlbrite-lint/src/main/java/com/squareup/sqlbrite3/SqlBriteArgCountDetector.kt create mode 100644 sqlbrite-lint/src/test/java/com/squareup/sqlbrite3/SqlBriteArgCountDetectorTest.kt diff --git a/build.gradle b/build.gradle index ac4455bb..a0bb6738 100644 --- a/build.gradle +++ b/build.gradle @@ -3,6 +3,7 @@ buildscript { 'minSdk': 14, 'compileSdk': 27, 'kotlin': '1.1.60', + 'lint': '26.0.1' ] repositories { @@ -21,6 +22,7 @@ allprojects { repositories { mavenCentral() google() + jcenter() } group = GROUP @@ -50,6 +52,11 @@ ext { rxBinding = 'com.jakewharton.rxbinding2:rxbinding:2.0.0' junit = 'junit:junit:4.12' truth = 'com.google.truth:truth:0.36' + + // Lint dependencies. + lintApi = "com.android.tools.lint:lint-api:${versions.lint}" + lint = "com.android.tools.lint:lint:${versions.lint}" + lintTests = "com.android.tools.lint:lint-tests:${versions.lint}" } configurations { diff --git a/settings.gradle b/settings.gradle index 445958a1..c8e57996 100644 --- a/settings.gradle +++ b/settings.gradle @@ -1,5 +1,6 @@ include ':sqlbrite' include ':sqlbrite-kotlin' +include ':sqlbrite-lint' include ':sample' rootProject.name = 'sqlbrite-root' diff --git a/sqlbrite-lint/build.gradle b/sqlbrite-lint/build.gradle new file mode 100644 index 00000000..6760c202 --- /dev/null +++ b/sqlbrite-lint/build.gradle @@ -0,0 +1,16 @@ +apply plugin: 'kotlin' + +dependencies { + compileOnly rootProject.ext.kotlinStdLib + compileOnly rootProject.ext.lintApi + + testImplementation rootProject.ext.junit + testImplementation rootProject.ext.lint + testImplementation rootProject.ext.lintTests +} + +jar { + manifest { + attributes("Lint-Registry-v2": "com.squareup.sqlbrite3.BriteIssueRegistry") + } +} diff --git a/sqlbrite-lint/src/main/java/com/squareup/sqlbrite3/BriteIssueRegistry.kt b/sqlbrite-lint/src/main/java/com/squareup/sqlbrite3/BriteIssueRegistry.kt new file mode 100644 index 00000000..ae915519 --- /dev/null +++ b/sqlbrite-lint/src/main/java/com/squareup/sqlbrite3/BriteIssueRegistry.kt @@ -0,0 +1,23 @@ +/* + * Copyright (C) 2017 Square, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.squareup.sqlbrite3 + +import com.android.tools.lint.client.api.IssueRegistry + +class BriteIssueRegistry : IssueRegistry() { + + override fun getIssues() = listOf(SqlBriteArgCountDetector.ISSUE) +} diff --git a/sqlbrite-lint/src/main/java/com/squareup/sqlbrite3/SqlBriteArgCountDetector.kt b/sqlbrite-lint/src/main/java/com/squareup/sqlbrite3/SqlBriteArgCountDetector.kt new file mode 100644 index 00000000..a3535632 --- /dev/null +++ b/sqlbrite-lint/src/main/java/com/squareup/sqlbrite3/SqlBriteArgCountDetector.kt @@ -0,0 +1,79 @@ +/* + * Copyright (C) 2017 Square, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.squareup.sqlbrite3 + +import com.android.tools.lint.detector.api.Category +import com.android.tools.lint.detector.api.ConstantEvaluator.evaluateString +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Implementation +import com.android.tools.lint.detector.api.Issue +import com.android.tools.lint.detector.api.JavaContext +import com.android.tools.lint.detector.api.Scope.JAVA_FILE +import com.android.tools.lint.detector.api.Scope.TEST_SOURCES +import com.android.tools.lint.detector.api.Severity +import com.intellij.psi.PsiMethod +import org.jetbrains.uast.UCallExpression +import java.util.EnumSet + +private const val BRITE_DATABASE = "com.squareup.sqlbrite3.BriteDatabase" +private const val QUERY_METHOD_NAME = "query" +private const val CREATE_QUERY_METHOD_NAME = "createQuery" + +class SqlBriteArgCountDetector : Detector(), Detector.UastScanner { + + companion object { + + val ISSUE: Issue = Issue.create( + "SqlBriteArgCount", + "Number of provided arguments doesn't match number " + + "of arguments specified in query", + "When providing arguments to query you need to provide the same amount of " + + "arguments that is specified in query.", + Category.MESSAGES, + 9, + Severity.ERROR, + Implementation(SqlBriteArgCountDetector::class.java, EnumSet.of(JAVA_FILE, TEST_SOURCES))) + } + + override fun getApplicableMethodNames() = listOf(CREATE_QUERY_METHOD_NAME, QUERY_METHOD_NAME) + + override fun visitMethod(context: JavaContext, call: UCallExpression, method: PsiMethod) { + val evaluator = context.evaluator + + if (evaluator.isMemberInClass(method, BRITE_DATABASE)) { + // Skip non varargs overloads. + if (!method.isVarArgs) return + + // Position of sql parameter depends on method. + val sql = evaluateString(context, + call.valueArguments[if (call.isQueryMethod()) 0 else 1], true) ?: return + + // Count only vararg arguments. + val argumentsCount = call.valueArgumentCount - if (call.isQueryMethod()) 1 else 2 + val questionMarksCount = sql.count { it == '?' } + if (argumentsCount != questionMarksCount) { + val requiredArguments = "$questionMarksCount ${"argument".pluralize(questionMarksCount)}" + val actualArguments = "$argumentsCount ${"argument".pluralize(argumentsCount)}" + context.report(ISSUE, call, context.getLocation(call), "Wrong argument count, " + + "query $sql requires $requiredArguments, but was provided $actualArguments") + } + } + } + + private fun UCallExpression.isQueryMethod() = methodName == QUERY_METHOD_NAME + + private fun String.pluralize(count: Int) = if (count == 1) this else this + "s" +} \ No newline at end of file diff --git a/sqlbrite-lint/src/test/java/com/squareup/sqlbrite3/SqlBriteArgCountDetectorTest.kt b/sqlbrite-lint/src/test/java/com/squareup/sqlbrite3/SqlBriteArgCountDetectorTest.kt new file mode 100644 index 00000000..c946983e --- /dev/null +++ b/sqlbrite-lint/src/test/java/com/squareup/sqlbrite3/SqlBriteArgCountDetectorTest.kt @@ -0,0 +1,207 @@ +/* + * Copyright (C) 2017 Square, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.squareup.sqlbrite3 + +import com.android.tools.lint.checks.infrastructure.TestFiles.java +import com.android.tools.lint.checks.infrastructure.TestLintTask.lint +import org.junit.Test + +class SqlBriteArgCountDetectorTest { + + companion object { + private val BRITE_DATABASE_STUB = java( + """ + package com.squareup.sqlbrite3; + + public final class BriteDatabase { + + public void query(String sql, Object... args) { + } + + public void createQuery(String table, String sql, Object... args) { + } + + // simulate createQuery with SupportSQLiteQuery query parameter + public void createQuery(String table, int something) { + } + } + """.trimIndent() + ) + } + + @Test + fun cleanCaseWithWithQueryAsLiteral() { + lint().files( + BRITE_DATABASE_STUB, + java( + """ + package test.pkg; + + import com.squareup.sqlbrite3.BriteDatabase; + + public class Test { + private static final String QUERY = "SELECT name FROM table WHERE id = ?"; + + public void test() { + BriteDatabase db = new BriteDatabase(); + db.query(QUERY, "id"); + } + + } + """.trimIndent())) + .issues(SqlBriteArgCountDetector.ISSUE) + .run() + .expectClean() + } + + @Test + fun cleanCaseWithQueryAsBinaryExpression() { + lint().files( + BRITE_DATABASE_STUB, + java( + """ + package test.pkg; + + import com.squareup.sqlbrite3.BriteDatabase; + + public class Test { + private static final String QUERY = "SELECT name FROM table WHERE "; + + public void test() { + BriteDatabase db = new BriteDatabase(); + db.query(QUERY + "id = ?", "id"); + } + + } + """.trimIndent())) + .issues(SqlBriteArgCountDetector.ISSUE) + .run() + .expectClean() + } + + @Test + fun cleanCaseWithQueryThatCantBeEvaluated() { + lint().files( + BRITE_DATABASE_STUB, + java( + """ + package test.pkg; + + import com.squareup.sqlbrite3.BriteDatabase; + + public class Test { + private static final String QUERY = "SELECT name FROM table WHERE id = ?"; + + public void test() { + BriteDatabase db = new BriteDatabase(); + db.query(query(), "id"); + } + + private String query() { + return QUERY + " age = ?"; + } + + } + """.trimIndent())) + .issues(SqlBriteArgCountDetector.ISSUE) + .run() + .expectClean() + } + + @Test + fun cleanCaseWithNonVarargMethodCall() { + lint().files( + BRITE_DATABASE_STUB, + java( + """ + package test.pkg; + + import com.squareup.sqlbrite3.BriteDatabase; + + public class Test { + + public void test() { + BriteDatabase db = new BriteDatabase(); + db.createQuery("table", 42); + } + + } + """.trimIndent())) + .issues(SqlBriteArgCountDetector.ISSUE) + .run() + .expectClean() + } + + @Test + fun queryMethodWithWrongNumberOfArguments() { + lint().files( + BRITE_DATABASE_STUB, + java( + """ + package test.pkg; + + import com.squareup.sqlbrite3.BriteDatabase; + + public class Test { + private static final String QUERY = "SELECT name FROM table WHERE id = ?"; + + public void test() { + BriteDatabase db = new BriteDatabase(); + db.query(QUERY); + } + + } + """.trimIndent())) + .issues(SqlBriteArgCountDetector.ISSUE) + .run() + .expect("src/test/pkg/Test.java:10: " + + "Error: Wrong argument count, query SELECT name FROM table WHERE id = ?" + + " requires 1 argument, but was provided 0 arguments [SqlBriteArgCount]\n" + + " db.query(QUERY);\n" + + " ~~~~~~~~~~~~~~~\n" + + "1 errors, 0 warnings") + } + + @Test + fun createQueryMethodWithWrongNumberOfArguments() { + lint().files( + BRITE_DATABASE_STUB, + java( + """ + package test.pkg; + + import com.squareup.sqlbrite3.BriteDatabase; + + public class Test { + private static final String QUERY = "SELECT name FROM table WHERE id = ?"; + + public void test() { + BriteDatabase db = new BriteDatabase(); + db.createQuery("table", QUERY); + } + + } + """.trimIndent())) + .issues(SqlBriteArgCountDetector.ISSUE) + .run() + .expect("src/test/pkg/Test.java:10: " + + "Error: Wrong argument count, query SELECT name FROM table WHERE id = ?" + + " requires 1 argument, but was provided 0 arguments [SqlBriteArgCount]\n" + + " db.createQuery(\"table\", QUERY);\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "1 errors, 0 warnings") + } +} \ No newline at end of file diff --git a/sqlbrite/build.gradle b/sqlbrite/build.gradle index 87b23f41..229e65da 100644 --- a/sqlbrite/build.gradle +++ b/sqlbrite/build.gradle @@ -8,6 +8,8 @@ dependencies { androidTestImplementation rootProject.ext.supportTestRunner androidTestImplementation rootProject.ext.truth androidTestImplementation rootProject.ext.supportSqliteFramework + + lintChecks project(':sqlbrite-lint') } android {