diff options
Diffstat (limited to 'mobile/android/fenix/mozilla-lint-rules')
14 files changed, 1273 insertions, 0 deletions
diff --git a/mobile/android/fenix/mozilla-lint-rules/.gitignore b/mobile/android/fenix/mozilla-lint-rules/.gitignore new file mode 100644 index 0000000000..796b96d1c4 --- /dev/null +++ b/mobile/android/fenix/mozilla-lint-rules/.gitignore @@ -0,0 +1 @@ +/build diff --git a/mobile/android/fenix/mozilla-lint-rules/build.gradle b/mobile/android/fenix/mozilla-lint-rules/build.gradle new file mode 100644 index 0000000000..d0fd50d35e --- /dev/null +++ b/mobile/android/fenix/mozilla-lint-rules/build.gradle @@ -0,0 +1,32 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +apply plugin: 'java-library' +apply plugin: 'kotlin' + +repositories { + gradle.mozconfig.substs.GRADLE_MAVEN_REPOSITORIES.each { repository -> + maven { + url repository + if (gradle.mozconfig.substs.ALLOW_INSECURE_GRADLE_REPOSITORIES) { + allowInsecureProtocol = true + } + } + } +} + +dependencies { + compileOnly "com.android.tools.lint:lint-api:${Versions.lint}" + compileOnly "com.android.tools.lint:lint-checks:${Versions.lint}" + + testImplementation "junit:junit:4.13.2" + testImplementation "com.android.tools.lint:lint:${Versions.lint}" + testImplementation "com.android.tools.lint:lint-tests:${Versions.lint}" +} + +jar { + manifest { + attributes('Lint-Registry-v2': 'org.mozilla.fenix.lintrules.LintIssueRegistry') + } +} diff --git a/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/AndroidSrcXmlDetector.kt b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/AndroidSrcXmlDetector.kt new file mode 100644 index 0000000000..3becda8457 --- /dev/null +++ b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/AndroidSrcXmlDetector.kt @@ -0,0 +1,80 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.lintrules + +import com.android.SdkConstants.ATTR_SRC +import com.android.SdkConstants.FQCN_IMAGE_BUTTON +import com.android.SdkConstants.FQCN_IMAGE_VIEW +import com.android.SdkConstants.IMAGE_BUTTON +import com.android.SdkConstants.IMAGE_VIEW +import com.android.resources.ResourceFolderType +import com.android.tools.lint.detector.api.Category +import com.android.tools.lint.detector.api.Implementation +import com.android.tools.lint.detector.api.Issue +import com.android.tools.lint.detector.api.ResourceXmlDetector +import com.android.tools.lint.detector.api.Scope +import com.android.tools.lint.detector.api.Severity +import com.android.tools.lint.detector.api.XmlContext +import org.w3c.dom.Element + +/** + * A custom lint check that prohibits not using the app:srcCompat for ImageViews + */ +class AndroidSrcXmlDetector : ResourceXmlDetector() { + companion object { + const val SCHEMA = "http://schemas.android.com/apk/res/android" + const val FULLY_QUALIFIED_APP_COMPAT_IMAGE_BUTTON = + "androidx.appcompat.widget.AppCompatImageButton" + const val FULLY_QUALIFIED_APP_COMPAT_VIEW_CLASS = + "androidx.appcompat.widget.AppCompatImageView" + const val APP_COMPAT_IMAGE_BUTTON = "AppCompatImageButton" + const val APP_COMPAT_IMAGE_VIEW = "AppCompatImageView" + + const val ERROR_MESSAGE = "Using android:src to define resource instead of app:srcCompat" + + val ISSUE_XML_SRC_USAGE = Issue.create( + id = "AndroidSrcXmlDetector", + briefDescription = "Prohibits using android:src in ImageViews and ImageButtons", + explanation = "ImageView (and descendants) images should be declared using app:srcCompat", + category = Category.CORRECTNESS, + severity = Severity.ERROR, + implementation = Implementation( + AndroidSrcXmlDetector::class.java, + Scope.RESOURCE_FILE_SCOPE, + ), + ) + } + + override fun appliesTo(folderType: ResourceFolderType): Boolean { + // Return true if we want to analyze resource files in the specified resource + // folder type. In this case we only need to analyze layout resource files. + return folderType == ResourceFolderType.LAYOUT + } + + override fun getApplicableElements(): Collection<String>? { + return setOf( + FQCN_IMAGE_VIEW, + IMAGE_VIEW, + FQCN_IMAGE_BUTTON, + IMAGE_BUTTON, + FULLY_QUALIFIED_APP_COMPAT_IMAGE_BUTTON, + FULLY_QUALIFIED_APP_COMPAT_VIEW_CLASS, + APP_COMPAT_IMAGE_BUTTON, + APP_COMPAT_IMAGE_VIEW, + ) + } + + override fun visitElement(context: XmlContext, element: Element) { + if (!element.hasAttributeNS(SCHEMA, ATTR_SRC)) return + val node = element.getAttributeNodeNS(SCHEMA, ATTR_SRC) + + context.report( + issue = ISSUE_XML_SRC_USAGE, + scope = node, + location = context.getLocation(node), + message = ERROR_MESSAGE, + ) + } +} diff --git a/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/ButtonStyleXmlDetector.kt b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/ButtonStyleXmlDetector.kt new file mode 100644 index 0000000000..3a1a2006df --- /dev/null +++ b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/ButtonStyleXmlDetector.kt @@ -0,0 +1,64 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.lintrules + +import com.android.SdkConstants +import com.android.SdkConstants.ATTR_STYLE +import com.android.resources.ResourceFolderType +import com.android.tools.lint.detector.api.Category +import com.android.tools.lint.detector.api.Implementation +import com.android.tools.lint.detector.api.Issue +import com.android.tools.lint.detector.api.ResourceXmlDetector +import com.android.tools.lint.detector.api.Scope +import com.android.tools.lint.detector.api.Severity +import com.android.tools.lint.detector.api.XmlContext +import org.w3c.dom.Element + +/** + * A custom lint check that prohibits not using the style attribute on buttons + */ +class ButtonStyleXmlDetector : ResourceXmlDetector() { + companion object { + const val ERROR_MESSAGE = + "All buttons must have a style, try using NeutralButton or similar." + + val ISSUE_XML_STYLE = Issue.create( + id = "ButtonStyleXmlDetector", + briefDescription = "Prohibits using a button without a style", + explanation = "Butttons should have a style applied", + category = Category.CORRECTNESS, + severity = Severity.ERROR, + implementation = Implementation( + ButtonStyleXmlDetector::class.java, + Scope.RESOURCE_FILE_SCOPE, + ), + ) + } + + override fun appliesTo(folderType: ResourceFolderType): Boolean { + // Return true if we want to analyze resource files in the specified resource + // folder type. In this case we only need to analyze layout resource files. + return folderType == ResourceFolderType.LAYOUT + } + + override fun getApplicableElements(): Collection<String>? { + return setOf( + SdkConstants.FQCN_BUTTON, + SdkConstants.MATERIAL_BUTTON, + SdkConstants.BUTTON, + ) + } + + override fun visitElement(context: XmlContext, element: Element) { + if (element.hasAttribute(ATTR_STYLE)) { return } + + context.report( + issue = ISSUE_XML_STYLE, + scope = element, + location = context.getElementLocation(element), + message = ERROR_MESSAGE, + ) + } +} diff --git a/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/ContextCompatDetector.kt b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/ContextCompatDetector.kt new file mode 100644 index 0000000000..6ffdac6d5f --- /dev/null +++ b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/ContextCompatDetector.kt @@ -0,0 +1,102 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.lintrules + +import com.android.tools.lint.detector.api.* +import com.intellij.psi.PsiMethod +import org.jetbrains.uast.UCallExpression + +class ContextCompatDetector : Detector(), SourceCodeScanner { + + companion object { + + const val FULLY_QUALIFIED_CONTEXT_COMPAT = "androidx.core.content.ContextCompat" + + private val Implementation = Implementation( + ContextCompatDetector::class.java, + Scope.JAVA_FILE_SCOPE, + ) + + val ISSUE_GET_DRAWABLE_CALL = Issue.create( + id = "UnsafeCompatGetDrawable", + briefDescription = "Prohibits using the ContextCompat.getDrawable method", + explanation = "Using this method can lead to crashes in older Android versions as newer features might not be available", + category = Category.CORRECTNESS, + severity = Severity.ERROR, + implementation = Implementation, + ) + + val ISSUE_GET_COLOR_STATE_LIST_CALL = Issue.create( + id = "UnsafeCompatGetColorStateList", + briefDescription = "Prohibits using the ContextCompat.getColorStateList method", + explanation = "Using this method can lead to crashes in older Android versions as newer features might not be available", + category = Category.CORRECTNESS, + severity = Severity.ERROR, + implementation = Implementation, + ) + + val ISSUES = listOf( + ISSUE_GET_DRAWABLE_CALL, + ISSUE_GET_COLOR_STATE_LIST_CALL, + ) + } + + override fun getApplicableMethodNames(): List<String>? = listOf( + "getDrawable", + "getColorStateList", + ) + + override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) { + val evaluator = context.evaluator + + if (!evaluator.isMemberInClass(method, FULLY_QUALIFIED_CONTEXT_COMPAT)) { + return + } + + when (node.methodName) { + "getDrawable" -> reportGetDrawableCall(context, node) + "getColorStateList" -> reportGetColorStateListCall(context, node) + } + } + + private fun reportGetDrawableCall(context: JavaContext, node: UCallExpression) = context.report( + ISSUE_GET_DRAWABLE_CALL, + context.getLocation(node), + "This call can lead to crashes, replace with AppCompatResources.getDrawable", + replaceUnsafeGetDrawableQuickFix(node), + ) + + private fun reportGetColorStateListCall(context: JavaContext, node: UCallExpression) = + context.report( + ISSUE_GET_COLOR_STATE_LIST_CALL, + context.getLocation(node), + "This call can lead to crashes, replace with AppCompatResources.getColorStateList", + replaceUnsafeGetColorStateListCallQuickFix(node), + ) + + private fun replaceUnsafeGetDrawableQuickFix(node: UCallExpression): LintFix { + val arguments = node.valueArguments.joinToString { it.asSourceString() } + val newText = "AppCompatResources.getDrawable($arguments)" + + return LintFix.create() + .name("Replace with AppCompatResources.getDrawable") + .replace() + .all() + .with(newText) + .build() + } + + private fun replaceUnsafeGetColorStateListCallQuickFix(node: UCallExpression): LintFix { + val arguments = node.valueArguments.joinToString { it.asSourceString() } + val newText = "AppCompatResources.getColorStateList($arguments)" + + return LintFix.create() + .name("Replace with AppCompatResources.getColorStateList") + .replace() + .all() + .with(newText) + .build() + } +} diff --git a/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/ImageViewAndroidTintXmlDetector.kt b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/ImageViewAndroidTintXmlDetector.kt new file mode 100644 index 0000000000..4fdf0ec2fb --- /dev/null +++ b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/ImageViewAndroidTintXmlDetector.kt @@ -0,0 +1,81 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.lintrules + +import com.android.SdkConstants.ATTR_TINT +import com.android.SdkConstants.FQCN_IMAGE_BUTTON +import com.android.SdkConstants.FQCN_IMAGE_VIEW +import com.android.SdkConstants.IMAGE_BUTTON +import com.android.SdkConstants.IMAGE_VIEW +import com.android.resources.ResourceFolderType +import com.android.tools.lint.detector.api.Category +import com.android.tools.lint.detector.api.Implementation +import com.android.tools.lint.detector.api.Issue +import com.android.tools.lint.detector.api.ResourceXmlDetector +import com.android.tools.lint.detector.api.Scope +import com.android.tools.lint.detector.api.Severity +import com.android.tools.lint.detector.api.XmlContext +import org.w3c.dom.Element + +/** + * A custom lint check that prohibits not using the app:tint for ImageViews + */ +class ImageViewAndroidTintXmlDetector : ResourceXmlDetector() { + companion object { + const val SCHEMA = "http://schemas.android.com/apk/res/android" + const val FULLY_QUALIFIED_APP_COMPAT_IMAGE_BUTTON = + "androidx.appcompat.widget.AppCompatImageButton" + const val FULLY_QUALIFIED_APP_COMPAT_VIEW_CLASS = + "androidx.appcompat.widget.AppCompatImageView" + const val APP_COMPAT_IMAGE_BUTTON = "AppCompatImageButton" + const val APP_COMPAT_IMAGE_VIEW = "AppCompatImageView" + + const val ERROR_MESSAGE = + "Using android:tint to tint ImageView instead of app:tint with AppCompatImageView" + + val ISSUE_XML_SRC_USAGE = Issue.create( + id = "AndroidSrcXmlDetector", + briefDescription = "Prohibits using android:tint in ImageViews and ImageButtons", + explanation = "ImageView (and descendants) should be tinted using app:tint", + category = Category.CORRECTNESS, + severity = Severity.ERROR, + implementation = Implementation( + ImageViewAndroidTintXmlDetector::class.java, + Scope.RESOURCE_FILE_SCOPE, + ), + ) + } + + override fun appliesTo(folderType: ResourceFolderType): Boolean { + // Return true if we want to analyze resource files in the specified resource + // folder type. In this case we only need to analyze layout resource files. + return folderType == ResourceFolderType.LAYOUT + } + + override fun getApplicableElements(): Collection<String>? { + return setOf( + FQCN_IMAGE_VIEW, + IMAGE_VIEW, + FQCN_IMAGE_BUTTON, + IMAGE_BUTTON, + FULLY_QUALIFIED_APP_COMPAT_IMAGE_BUTTON, + FULLY_QUALIFIED_APP_COMPAT_VIEW_CLASS, + APP_COMPAT_IMAGE_BUTTON, + APP_COMPAT_IMAGE_VIEW, + ) + } + + override fun visitElement(context: XmlContext, element: Element) { + if (!element.hasAttributeNS(SCHEMA, ATTR_TINT)) return + val node = element.getAttributeNodeNS(SCHEMA, ATTR_TINT) + + context.report( + issue = ISSUE_XML_SRC_USAGE, + scope = node, + location = context.getLocation(node), + message = ERROR_MESSAGE, + ) + } +} diff --git a/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/LicenseCommentChecker.kt b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/LicenseCommentChecker.kt new file mode 100644 index 0000000000..efc4b938d2 --- /dev/null +++ b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/LicenseCommentChecker.kt @@ -0,0 +1,84 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.lintrules + +import com.android.tools.lint.client.api.UElementHandler +import com.android.tools.lint.detector.api.JavaContext +import com.android.tools.lint.detector.api.LintFix +import org.jetbrains.kotlin.psi.psiUtil.siblings +import org.jetbrains.kotlin.psi.psiUtil.startsWithComment +import org.jetbrains.uast.UComment +import org.jetbrains.uast.UFile + +class LicenseCommentChecker(private val context: JavaContext) : UElementHandler() { + + companion object { + val ValidLicenseForKotlinFiles = """ + |/* This Source Code Form is subject to the terms of the Mozilla Public + | * License, v. 2.0. If a copy of the MPL was not distributed with this + | * file, You can obtain one at http://mozilla.org/MPL/2.0/. */""".trimMargin() + } + + override fun visitFile(node: UFile) { + if (!node.sourcePsi.startsWithComment()) { + reportMissingLicense(node) + } else { + val firstComment = node.allCommentsInFile.first() + if (firstComment.text != ValidLicenseForKotlinFiles) { + reportInvalidLicenseFormat(firstComment) + } else { + val nextSibling = + firstComment.sourcePsi.siblings(withItself = false).firstOrNull() + if (nextSibling?.text != "\n\n") { + reportMissingLeadingNewLineCharacter(firstComment) + } + } + } + } + + private fun reportMissingLicense(node: UFile) = context.report( + LicenseDetector.ISSUE_MISSING_LICENSE, + context.getLocation(node.sourcePsi.firstChild), + "The file must start with a comment containing the license", + addLicenseQuickFix(), + ) + + private fun reportInvalidLicenseFormat(comment: UComment) = context.report( + LicenseDetector.ISSUE_INVALID_LICENSE_FORMAT, + context.getLocation(comment), + "The license comment doesn't have the appropriate format", + replaceCommentWithValidLicenseFix(comment), + ) + + private fun reportMissingLeadingNewLineCharacter(licenseComment: UComment) = context.report( + LicenseDetector.ISSUE_INVALID_LICENSE_FORMAT, + context.getRangeLocation(licenseComment, licenseComment.text.lastIndex, 1), + "The license comment must be followed by a newline character", + addLeadingNewLineQuickFix(licenseComment), + ) + + private fun addLicenseQuickFix() = LintFix.create() + .name("Insert license at the top of the file") + .replace() + .beginning() + .with(ValidLicenseForKotlinFiles + "\n\n") + .autoFix() + .build() + + private fun replaceCommentWithValidLicenseFix(comment: UComment) = LintFix.create() + .name("Replace with correctly formatted license") + .replace() + .range(context.getLocation(comment)) + .with(ValidLicenseForKotlinFiles) + .build() + + private fun addLeadingNewLineQuickFix(licenseComment: UComment) = LintFix.create() + .name("Insert newline character after the license") + .replace() + .range(context.getLocation(licenseComment)) + .with(ValidLicenseForKotlinFiles + "\n") + .autoFix() + .build() +} diff --git a/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/LicenseDetector.kt b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/LicenseDetector.kt new file mode 100644 index 0000000000..81c6b6552f --- /dev/null +++ b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/LicenseDetector.kt @@ -0,0 +1,45 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.lintrules + +import com.android.tools.lint.client.api.UElementHandler +import com.android.tools.lint.detector.api.* +import org.jetbrains.uast.UElement +import org.jetbrains.uast.UFile + +class LicenseDetector : Detector(), SourceCodeScanner { + + companion object { + + private val Implementation = Implementation( + LicenseDetector::class.java, + Scope.JAVA_FILE_SCOPE, + ) + + val ISSUE_MISSING_LICENSE = Issue.create( + id = "MissingLicense", + briefDescription = "File doesn't start with the license comment", + explanation = "Every file must start with the license comment:\n" + + LicenseCommentChecker.ValidLicenseForKotlinFiles, + category = Category.CORRECTNESS, + severity = Severity.WARNING, + implementation = Implementation, + ) + + val ISSUE_INVALID_LICENSE_FORMAT = Issue.create( + id = "InvalidLicenseFormat", + briefDescription = "License isn't formatted correctly", + explanation = "The license must be:\n${LicenseCommentChecker.ValidLicenseForKotlinFiles}", + category = Category.CORRECTNESS, + severity = Severity.WARNING, + implementation = Implementation, + ) + } + + override fun getApplicableUastTypes(): List<Class<out UElement>>? = listOf(UFile::class.java) + + override fun createUastHandler(context: JavaContext): UElementHandler? = + LicenseCommentChecker(context) +} diff --git a/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/LintIssueRegistry.kt b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/LintIssueRegistry.kt new file mode 100644 index 0000000000..86c70b80e1 --- /dev/null +++ b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/LintIssueRegistry.kt @@ -0,0 +1,27 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.lintrules + +import com.android.tools.lint.client.api.IssueRegistry +import com.android.tools.lint.client.api.Vendor +import com.android.tools.lint.detector.api.Issue +import org.mozilla.fenix.lintrules.perf.ConstraintLayoutPerfDetector + +/** + * Registry which provides a list of our custom lint checks to be performed on an Android project. + */ +@Suppress("unused") +class LintIssueRegistry : IssueRegistry() { + override val api: Int = com.android.tools.lint.detector.api.CURRENT_API + override val issues: List<Issue> = listOf( + ButtonStyleXmlDetector.ISSUE_XML_STYLE, + LicenseDetector.ISSUE_MISSING_LICENSE, + LicenseDetector.ISSUE_INVALID_LICENSE_FORMAT, + ) + ConstraintLayoutPerfDetector.ISSUES + ContextCompatDetector.ISSUES + override val vendor: Vendor = Vendor( + vendorName = "Mozilla", + identifier = "mozilla-fenix", + ) +} diff --git a/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/TextViewAndroidSrcXmlDetector.kt b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/TextViewAndroidSrcXmlDetector.kt new file mode 100644 index 0000000000..0ec8d690cb --- /dev/null +++ b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/TextViewAndroidSrcXmlDetector.kt @@ -0,0 +1,97 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.lintrules + +import com.android.SdkConstants.ATTR_DRAWABLE_BOTTOM +import com.android.SdkConstants.ATTR_DRAWABLE_END +import com.android.SdkConstants.ATTR_DRAWABLE_LEFT +import com.android.SdkConstants.ATTR_DRAWABLE_RIGHT +import com.android.SdkConstants.ATTR_DRAWABLE_START +import com.android.SdkConstants.ATTR_DRAWABLE_TOP +import com.android.SdkConstants.FQCN_TEXT_VIEW +import com.android.SdkConstants.TEXT_VIEW +import com.android.resources.ResourceFolderType +import com.android.tools.lint.detector.api.Category +import com.android.tools.lint.detector.api.Implementation +import com.android.tools.lint.detector.api.Issue +import com.android.tools.lint.detector.api.ResourceXmlDetector +import com.android.tools.lint.detector.api.Scope +import com.android.tools.lint.detector.api.Severity +import com.android.tools.lint.detector.api.XmlContext +import org.w3c.dom.Element + +/** + * A custom lint check that prohibits not using the app:drawableXCompat to define drawables in TextViews + */ +class TextViewAndroidSrcXmlDetector : ResourceXmlDetector() { + companion object { + const val SCHEMA = "http://schemas.android.com/apk/res/android" + + const val ERROR_MESSAGE = + "Using android:drawableX to define resource instead of app:drawableXCompat" + + val ISSUE_XML_SRC_USAGE = Issue.create( + id = "TextViewAndroidSrcXmlDetector", + briefDescription = "Prohibits using android namespace to define drawables in TextViews", + explanation = "TextView drawables should be declared using app:drawableXCompat", + category = Category.CORRECTNESS, + severity = Severity.ERROR, + implementation = Implementation( + TextViewAndroidSrcXmlDetector::class.java, + Scope.RESOURCE_FILE_SCOPE, + ), + ) + } + + override fun appliesTo(folderType: ResourceFolderType): Boolean { + // Return true if we want to analyze resource files in the specified resource + // folder type. In this case we only need to analyze layout resource files. + return folderType == ResourceFolderType.LAYOUT + } + + override fun getApplicableElements(): Collection<String>? { + return setOf( + FQCN_TEXT_VIEW, + TEXT_VIEW, + ) + } + + override fun visitElement(context: XmlContext, element: Element) { + val node = when { + element.hasAttributeNS(SCHEMA, ATTR_DRAWABLE_BOTTOM) -> element.getAttributeNodeNS( + SCHEMA, + ATTR_DRAWABLE_BOTTOM, + ) + element.hasAttributeNS(SCHEMA, ATTR_DRAWABLE_END) -> element.getAttributeNodeNS( + SCHEMA, + ATTR_DRAWABLE_END, + ) + element.hasAttributeNS(SCHEMA, ATTR_DRAWABLE_LEFT) -> element.getAttributeNodeNS( + SCHEMA, + ATTR_DRAWABLE_LEFT, + ) + element.hasAttributeNS( + SCHEMA, + ATTR_DRAWABLE_RIGHT, + ) -> element.getAttributeNodeNS(SCHEMA, ATTR_DRAWABLE_RIGHT) + element.hasAttributeNS( + SCHEMA, + ATTR_DRAWABLE_START, + ) -> element.getAttributeNodeNS(SCHEMA, ATTR_DRAWABLE_START) + element.hasAttributeNS(SCHEMA, ATTR_DRAWABLE_TOP) -> element.getAttributeNodeNS( + SCHEMA, + ATTR_DRAWABLE_TOP, + ) + else -> null + } ?: return + + context.report( + issue = ISSUE_XML_SRC_USAGE, + scope = node, + location = context.getLocation(node), + message = ERROR_MESSAGE, + ) + } +} diff --git a/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/perf/ConstraintLayoutPerfDetector.kt b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/perf/ConstraintLayoutPerfDetector.kt new file mode 100644 index 0000000000..d7e08b57a8 --- /dev/null +++ b/mobile/android/fenix/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/perf/ConstraintLayoutPerfDetector.kt @@ -0,0 +1,225 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.lintrules.perf + +import com.android.resources.ResourceFolderType +import com.android.tools.lint.detector.api.Category +import com.android.tools.lint.detector.api.Context +import com.android.tools.lint.detector.api.Implementation +import com.android.tools.lint.detector.api.Issue +import com.android.tools.lint.detector.api.ResourceXmlDetector +import com.android.tools.lint.detector.api.Scope +import com.android.tools.lint.detector.api.Severity +import com.android.tools.lint.detector.api.XmlContext +import org.w3c.dom.Element + +private val FRAMEWORK_ELEMENTS = setOf( + "androidx.constraintlayout.widget.ConstraintLayout", + + // Android framework views that extend ConstraintLayout + "androidx.constraintlayout.motion.widget.MotionLayout", +) +private val FRAMEWORK_ELEMENTS_BULLETED_LIST = FRAMEWORK_ELEMENTS.map { "- `$it`" }.joinToString("\n") + +private const val FENIX_PREFIX = "org.mozilla.fenix" +private const val AC_PREFIX = "mozilla.components" + +/** + * Custom views that we wrote that extend ConstraintLayout. + * + * This data was manually added from the Type Hierarchy IDE action. Last update: 8/5/2020. + * In the future, we could generate this list statically to prevent transcription errors and + * errors from refactors. + */ +private val CUSTOM_VIEW_ELEMENTS = setOf( + "$FENIX_PREFIX.library.LibrarySiteItemView", + "$FENIX_PREFIX.settings.deletebrowsingdata.DeleteBrowsingDataItem", + "$FENIX_PREFIX.trackingprotection.SwitchWithDescription", + "$FENIX_PREFIX.trackingprotection.TrackingProtectionCategoryItem", + + "$AC_PREFIX.feature.readerview.view.ReaderViewControlsBar", + "$AC_PREFIX.feature.findinpage.view.FindInPageBar", + "$AC_PREFIX.feature.prompts.login.LoginSelectBar", + "$AC_PREFIX.ui.widgets.WidgetSiteItemView", + "$AC_PREFIX.browser.toolbar.display.DisplayToolbarView", +) +private val CUSTOM_VIEW_ELEMENTS_BULLETED_LIST = CUSTOM_VIEW_ELEMENTS.map { "- `$it`" }.joinToString("\n") + +private val ALL_APPLICABLE_ELEMENTS = FRAMEWORK_ELEMENTS + CUSTOM_VIEW_ELEMENTS + +private const val FRAMEWORK_ISSUE_DESCRIPTION = "2+ ConstraintLayouts in one hierarchy can be inefficient" +private const val FRAMEWORK_ISSUE_MESSAGE = "Flatten the view hierarchy by using one `ConstraintLayout`, " + + "if possible. If the alternative is several nested `ViewGroup`, it may not help performance and " + + "this may be worth suppressing." +private val FRAMEWORK_ISSUE_EXPLANATION = """`ConstraintLayout`'s main performance benefit is + that it enables devs to define layouts with very little view nesting. However, they are slow to + inflate so we only want to use as many as are strictly necessary. In most cases, this is one: a + layout can have a ConstraintLayout defined at the root and the entire hierarchy can be a direct + descendant of it. + + This check will report if there is more than one ConstraintLayout, or a view that extends it, + defined in a single layout file to remind the implementor of these performance concerns. + + **WARNING: replacing a ConstraintLayout with several nested ViewGroups is unlikely to improve + performance. Remember, a goal is to reduce nesting. If you're unable to remove the extra + ConstraintLayout(s) without doing this, it's probably better to suppress this violation.** + + Here are **known cases where ConstraintLayouts are unnecessarily nested and the alternatives:** + - To set a background color. Instead, use a `<view>` of the appropriate size directly in the ConstraintLayout + - To change visibility on a group of sub-views. Instead, use `androidx.constraintlayout.widget.Group` + + There are a few cases the perf team is not 100% sure of the best solution - let us know if you + have better solutions: + - ScrollView can only have one child so perhaps making it a ConstraintLayout is fine, despite nesting + - a11y may rely on nesting views. Maybe `Group` solves this problem? + + The following views that are, or that extend, ConstraintLayout are inspected in this check: +""".trimIndent() + .lines() + .joinToStringRemovingManualWrapping() + + "\n$FRAMEWORK_ELEMENTS_BULLETED_LIST\n\n" + +private val CUSTOM_VIEW_ISSUE_DESCRIPTION = "Caution regarding custom views extending `ConstraintLayout`" +private val CUSTOM_VIEW_ISSUE_MESSAGE = "Custom views extending `ConstraintLayout` are less " + + "efficient because they cannot share other `ConstraintLayout` defined in file." +private val CUSTOM_VIEW_ISSUE_EXPLANATION = """`ConstraintLayout` is slow to inflate so we + should aim to have as few as possible. However, the performance team doesn't have a good generic + solution for when custom views extend ConstraintLayout because the encapsulation may be worth + it and the replacement may not be more performant. Therefore, we warn the implementor that this + may be undesireable but don't fail the build on it. + + More generic information about the performance concerns of ConstraintLayout is found below, + copied from our more strict ConstraintLayout check. + + The following additional views are inspected for this less strict check: +""".trimIndent() + .lines() + .joinToStringRemovingManualWrapping() + + "\n$CUSTOM_VIEW_ELEMENTS_BULLETED_LIST\n\n" + + "---\n\n" + // divider + FRAMEWORK_ISSUE_EXPLANATION + +/** + * An issue where multiple ConstraintLayouts that are defined by the framework are detected in the + * same file. This is distinct from [CUSTOM_VIEW_ISSUE] because that one handles custom views. + * We don't want to combine them because solving a custom view ConstraintLayout problem rarely has + * a clear cut solution and we don't want to fail the build on checks that aren't clear cut. + */ +private val FRAMEWORK_ISSUE = Issue.create( + id = "MozMultipleConstraintLayouts", + briefDescription = FRAMEWORK_ISSUE_DESCRIPTION, + explanation = FRAMEWORK_ISSUE_EXPLANATION, + category = Category.PERFORMANCE, + priority = 8, // UseCompoundDrawables is a 6 and I think this is much more impactful. + severity = Severity.ERROR, + implementation = Implementation( + ConstraintLayoutPerfDetector::class.java, + Scope.RESOURCE_FILE_SCOPE, + ), +) + +/** + * An issue where multiple ConstraintLayouts, that may also be defined by us as custom views, are + * detected in the same file. See [FRAMEWORK_ISSUE] for why these are distinct. + */ +private val CUSTOM_VIEW_ISSUE = Issue.create( + id = "MozMultipleConstraintLayoutsAndCustomViews", + briefDescription = CUSTOM_VIEW_ISSUE_DESCRIPTION, + explanation = CUSTOM_VIEW_ISSUE_EXPLANATION, + category = Category.PERFORMANCE, + priority = 6, // UseCompoundDrawables is a 6 and this seems more impactful but less actionable. + severity = Severity.WARNING, + implementation = Implementation( + ConstraintLayoutPerfDetector::class.java, + Scope.RESOURCE_FILE_SCOPE, + ), +) + +/** + * A custom lint detector that helps us ensure we're using ConstraintLayout performantly. + * + * This detector has a few shortcomings: + * - it will not notice if a ConstraintLayout is in the same view hierarchy at runtime but added + * from a different file (e.g. via <include>) + * - it will not notice if a ConstraintLayout is added at runtime + * - if we add additional views that extend ConstraintLayout, this check won't know about them + * (though we could generate this information and include it in this check via static analysis) + */ +class ConstraintLayoutPerfDetector : ResourceXmlDetector() { + + companion object { + val ISSUES = listOf( + FRAMEWORK_ISSUE, + CUSTOM_VIEW_ISSUE, + ) + } + + private var frameworkConstraintLayoutCountInFile = 0 + private var customViewConstraintLayoutCountInFile = 0 + + override fun appliesTo(folderType: ResourceFolderType): Boolean = folderType == ResourceFolderType.LAYOUT + override fun getApplicableElements(): Collection<String>? = ALL_APPLICABLE_ELEMENTS + + override fun beforeCheckFile(context: Context) { + frameworkConstraintLayoutCountInFile = 0 + customViewConstraintLayoutCountInFile = 0 + } + + override fun visitElement(context: XmlContext, element: Element) { + // This scope is unideal: if the root element is a ConstraintLayout and is suppressed, all + // ConstraintLayout children will also be suppressed. If more ConstraintLayout children are + // added, the root ConstraintLayout will suppress them too. Ideally, we'd want a suppression + // to only affect a node and not its children. + val scope = element + val location = context.getElementLocation(element) + + // For these inner visit methods, we only warn on 2+ violations because: + // - we avoid having 2 warnings for the first intuitive violation (i.e. 2 ConstraintLayouts + // in a file is one violation on the second ConstraintLayout) + // - of the suppression & scope issues mentioned above. The first found element is most + // likely to be a parent of other ConstraintLayouts because of the traversal order so if + // there's one node we don't report on, that's the best one. + // + // The fact that we use two different issues can create a weird situation where if a custom + // view is a sibling above a ConstraintLayout, it won't report but if it's below the + // ConstraintLayout, it will report. I don't think it's worth the complexity to address this + // and in-IDE inspection seems to break when we use afterCheckFile, which would be useful for this. + fun visitFrameworkElement() { + frameworkConstraintLayoutCountInFile += 1 + if (frameworkConstraintLayoutCountInFile > 1) { + context.report(FRAMEWORK_ISSUE, scope, location, FRAMEWORK_ISSUE_MESSAGE) + } + } + + fun visitCustomViewElement() { + customViewConstraintLayoutCountInFile += 1 + + // We want to report if there are too many ConstraintLayouts in the file. This issue + // intentionally includes both framework CL and our custom view CL so we add both counts. + if (frameworkConstraintLayoutCountInFile + customViewConstraintLayoutCountInFile > 1) { + context.report(CUSTOM_VIEW_ISSUE, scope, location, CUSTOM_VIEW_ISSUE_MESSAGE) + } + } + + when (element.tagName) { + in FRAMEWORK_ELEMENTS -> visitFrameworkElement() + in CUSTOM_VIEW_ELEMENTS -> visitCustomViewElement() + else -> throw IllegalStateException("Unexpected tag seen: ${element.tagName}") + } + } +} + +/** + * Takes paragraphs wrapped by hand and combines them into a single line, preserving white space + * between paragraphs. + */ +private fun List<String>.joinToStringRemovingManualWrapping(): String = fold("") { acc, str -> + val toAppend = when { + str.isBlank() -> "\n\n" // new paragraph + str.trim().startsWith("-") -> "\n$str" // bulleted list + else -> str + } + acc + toAppend +} diff --git a/mobile/android/fenix/mozilla-lint-rules/src/test/java/org/mozilla/fenix/lintrules/ContextCompatDetectorTest.kt b/mobile/android/fenix/mozilla-lint-rules/src/test/java/org/mozilla/fenix/lintrules/ContextCompatDetectorTest.kt new file mode 100644 index 0000000000..9a8a7fa99c --- /dev/null +++ b/mobile/android/fenix/mozilla-lint-rules/src/test/java/org/mozilla/fenix/lintrules/ContextCompatDetectorTest.kt @@ -0,0 +1,163 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.lintrules + +import com.android.tools.lint.checks.infrastructure.LintDetectorTest +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Issue +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +@RunWith(JUnit4::class) +class ContextCompatDetectorTest : LintDetectorTest() { + + override fun getDetector(): Detector = ContextCompatDetector() + + override fun getIssues(): MutableList<Issue> = ContextCompatDetector.ISSUES.toMutableList() + + @Test + fun `report error when the ContextCompat getDrawable method is called`() { + val code = """ + |package example + | + |import androidx.core.content.ContextCompat + |import android.content.Context + | + |class Fake { + | + | fun trigger(context: Context, id: Int) { + | ContextCompat.getDrawable(context, id) + | } + | + |}""".trimMargin() + + val expectedReport = """ + |src/example/Fake.kt:9: Error: This call can lead to crashes, replace with AppCompatResources.getDrawable [UnsafeCompatGetDrawable] + | ContextCompat.getDrawable(context, id) + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + |1 errors, 0 warnings""".trimMargin() + + val expectedFixOutput = """ + |Fix for src/example/Fake.kt line 9: Replace with AppCompatResources.getDrawable: + |@@ -9 +9 + |- ContextCompat.getDrawable(context, id) + |+ AppCompatResources.getDrawable(context, id)""".trimMargin() + + lint() + .files(Context, ContextCompat, kotlin(code)) + .allowMissingSdk(true) + .run() + .expect(expectedReport) + .expectFixDiffs(expectedFixOutput) + } + + @Test + fun `report error when the ContextCompat getColorStateList method is called`() { + val code = """ + |package example + | + |import androidx.core.content.ContextCompat + |import android.content.Context + | + |class Fake { + | + | fun trigger(context: Context, id: Int) { + | ContextCompat.getColorStateList(context, id) + | } + | + |}""".trimMargin() + + val expectedReport = """ + |src/example/Fake.kt:9: Error: This call can lead to crashes, replace with AppCompatResources.getColorStateList [UnsafeCompatGetColorStateList] + | ContextCompat.getColorStateList(context, id) + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + |1 errors, 0 warnings""".trimMargin() + + val expectedFixOutput = """ + |Fix for src/example/Fake.kt line 9: Replace with AppCompatResources.getColorStateList: + |@@ -9 +9 + |- ContextCompat.getColorStateList(context, id) + |+ AppCompatResources.getColorStateList(context, id)""".trimMargin() + + lint() + .files(Context, ContextCompat, kotlin(code)) + .allowMissingSdk(true) + .run() + .expect(expectedReport) + .expectFixDiffs(expectedFixOutput) + } + + @Test + fun `does not report any errors if the getDrawable method being called is not from the ContextCompat class`() { + val code = """ + |package example + | + |import android.content.Context + | + |class Fake { + | + | fun trigger(context: Context, id: Int) { + | getDrawable(context, id) + | } + | + | fun getDrawable(context: Context, id: Int) {} + | + |}""".trimMargin() + + lint() + .files(Context, kotlin(code)) + .allowMissingSdk(true) + .run() + .expectClean() + } + + @Test + fun `does not report any errors if the getColorStateList method being called is not from the ContextCompat class`() { + val code = """ + |package example + | + |import android.content.Context + | + |class Fake { + | + | fun trigger(context: Context, id: Int) { + | getColorStateList(context, id) + | } + | + | fun getColorStateList(context: Context, id: Int) {} + | + |}""".trimMargin() + + lint() + .files(Context, kotlin(code)) + .allowMissingSdk(true) + .run() + .expectClean() + } + + // As Lint doesn't have access to the real corresponding classes, we have to provide stubs + companion object Stubs { + + private val Context = java( + """ + |package android.content; + | + |public class Context {} + |""".trimMargin() + ) + + private val ContextCompat = java( + """ + |package androidx.core.content; + | + |public class ContextCompat { + | public static void getDrawable(Context context, int resId) {} + | public static void getColorStateList(Context context, int resId) {} + |} + |""".trimMargin() + ) + } +} diff --git a/mobile/android/fenix/mozilla-lint-rules/src/test/java/org/mozilla/fenix/lintrules/LicenseDetectorTest.kt b/mobile/android/fenix/mozilla-lint-rules/src/test/java/org/mozilla/fenix/lintrules/LicenseDetectorTest.kt new file mode 100644 index 0000000000..c05cda1634 --- /dev/null +++ b/mobile/android/fenix/mozilla-lint-rules/src/test/java/org/mozilla/fenix/lintrules/LicenseDetectorTest.kt @@ -0,0 +1,155 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.lintrules + +import com.android.tools.lint.checks.infrastructure.LintDetectorTest +import com.android.tools.lint.checks.infrastructure.TestFiles +import com.android.tools.lint.checks.infrastructure.TestMode +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Issue +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import org.mozilla.fenix.lintrules.LicenseCommentChecker.Companion.ValidLicenseForKotlinFiles +import org.mozilla.fenix.lintrules.LicenseDetector.Companion.ISSUE_INVALID_LICENSE_FORMAT +import org.mozilla.fenix.lintrules.LicenseDetector.Companion.ISSUE_MISSING_LICENSE + +@RunWith(JUnit4::class) +class LicenseDetectorTest : LintDetectorTest() { + + override fun getIssues(): MutableList<Issue> = + mutableListOf(ISSUE_MISSING_LICENSE, ISSUE_INVALID_LICENSE_FORMAT) + + override fun getDetector(): Detector = LicenseDetector() + + @Test + fun `report missing license if it isn't at the top of the file`() { + val code = """ + |package example + | + |class SomeExample { + | fun test() { + | val x = 10 + | println("Hello") + | } + |}""".trimMargin() + + val expectedReport = """ + |src/example/SomeExample.kt:1: Warning: The file must start with a comment containing the license [MissingLicense] + |package example + |~~~~~~~~~~~~~~~ + |0 errors, 1 warnings""".trimMargin() + + val expectedFixOutput = """ + |Fix for src/example/SomeExample.kt line 1: Insert license at the top of the file: + |@@ -1 +1 + |+ /* This Source Code Form is subject to the terms of the Mozilla Public + |+ * License, v. 2.0. If a copy of the MPL was not distributed with this + |+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + |+""".trimMargin() + + lint() + .files(TestFiles.kt(code)) + .allowMissingSdk(true) + .testModes(TestMode.UI_INJECTION_HOST) + .run() + .expect(expectedReport) + .expectFixDiffs(expectedFixOutput) + } + + @Test + fun `report invalid license format if it doesn't have a leading newline character`() { + val code = """ + |$ValidLicenseForKotlinFiles + |package example + | + |class SomeExample { + | fun test() { + | val x = 10 + | println("Hello") + | } + |}""".trimMargin() + + val expectedReport = """ + |src/example/SomeExample.kt:3: Warning: The license comment must be followed by a newline character [InvalidLicenseFormat] + | * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + | ~ + |0 errors, 1 warnings""".trimMargin() + + val expectedFixOutput = """ + |Fix for src/example/SomeExample.kt line 3: Insert newline character after the license: + |@@ -4 +4 + |+""".trimMargin() + + lint() + .files(TestFiles.kt(code)) + .allowMissingSdk(true) + .testModes(TestMode.UI_INJECTION_HOST) + .run() + .expect(expectedReport) + .expectFixDiffs(expectedFixOutput) + } + + @Test + fun `valid license does not generate a warning`() { + val code = """ + |$ValidLicenseForKotlinFiles + | + |package example + | + |class SomeExample { + | fun test() { + | val x = 10 + | println("Hello") + | } + |}""".trimMargin() + + lint() + .files(TestFiles.kt(code)) + .allowMissingSdk(true) + .testModes(TestMode.UI_INJECTION_HOST) + .run() + .expectClean() + } + + @Test + fun `report incorrectly formatted license`() { + val code = """ + |/* This Source Code Form is subject to the terms of the Mozilla Public + | License, v. 2.0. If a copy of the MPL was not distributed with this + | file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + | + |package example + | + |class SomeExample { + | fun test() { + | val x = 10 + | println("Hello") + | } + |}""".trimMargin() + + val expectedReport = """ + |src/example/SomeExample.kt:1: Warning: The license comment doesn't have the appropriate format [InvalidLicenseFormat] + |/* This Source Code Form is subject to the terms of the Mozilla Public + |^ + |0 errors, 1 warnings""".trimMargin() + + val expectedFixOutput = """ + |Fix for src/example/SomeExample.kt line 1: Replace with correctly formatted license: + |@@ -2 +2 + |- License, v. 2.0. If a copy of the MPL was not distributed with this + |- file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + |+ * License, v. 2.0. If a copy of the MPL was not distributed with this + |+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */""".trimMargin() + + lint() + .files(TestFiles.kt(code)) + .allowMissingSdk(true) + .testModes(TestMode.UI_INJECTION_HOST) + .run() + .expect(expectedReport) + .expectFixDiffs(expectedFixOutput) + } +} diff --git a/mobile/android/fenix/mozilla-lint-rules/src/test/java/org/mozilla/fenix/lintrules/perf/ConstraintLayoutPerfDetectorTest.kt b/mobile/android/fenix/mozilla-lint-rules/src/test/java/org/mozilla/fenix/lintrules/perf/ConstraintLayoutPerfDetectorTest.kt new file mode 100644 index 0000000000..1e1bacc1ed --- /dev/null +++ b/mobile/android/fenix/mozilla-lint-rules/src/test/java/org/mozilla/fenix/lintrules/perf/ConstraintLayoutPerfDetectorTest.kt @@ -0,0 +1,117 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.lintrules.perf + +import com.android.resources.ResourceFolderType +import com.android.tools.lint.checks.infrastructure.TestFiles +import com.android.tools.lint.checks.infrastructure.TestLintTask.lint +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Ignore +import org.junit.Test + +// FQCN = fully qualified class name +private const val FQCN_CONSTRAINT_LAYOUT = "androidx.constraintlayout.widget.ConstraintLayout" +private const val FQCN_CUSTOM_VIEW = "org.mozilla.fenix.library.LibrarySiteItemView" // chosen arbitrarily + +private val ALL_ISSUES = ConstraintLayoutPerfDetector.ISSUES.toTypedArray() + +class ConstraintLayoutPerfDetectorTest { + + private lateinit var detector: ConstraintLayoutPerfDetector + + @Before + fun setUp() { + detector = ConstraintLayoutPerfDetector() + } + + @Test + fun `WHEN checking file types THEN it only applies layout`() { + val excludedType = ResourceFolderType.LAYOUT + + val allTypeButLayout = ResourceFolderType.values().filter { it != excludedType } + allTypeButLayout.forEach { assertFalse(detector.appliesTo(it)) } + + assertTrue(detector.appliesTo(excludedType)) + } + + @Test + fun `WHEN checking applicable elements THEN it applies to ConstraintLayout` () { + assertTrue(detector.getApplicableElements()!!.contains( + "androidx.constraintlayout.widget.ConstraintLayout" + )) + } + + @Test + fun `WHEN checking applicable elements THEN it doesnt apply to some common ones`() { + val applicableElements = detector.getApplicableElements()!! + listOf( + "LinearLayout", + "fragment" + ).forEach { assertFalse(it, applicableElements.contains(it)) } + } + + @Test + fun `GIVEN a file with one ConstraintLayout WHEN detecting THEN pass`() { + val layout = getLayoutWithConstraintLayoutRoot("<TextView/>") + lint() + .files(getTestXml(layout)) + .issues(*ALL_ISSUES) + .run() + .expectClean() + } + + @Test + fun `GIVEN a file with two ConstraintLayouts WHEN detecting THEN fail`() { + // Since we're not the root view, we need to indent extra. + val innerLayout = """<$FQCN_CONSTRAINT_LAYOUT + | android:layout_width="match_parent" + | android:layout_height="match_parent"> + | + | </$FQCN_CONSTRAINT_LAYOUT> + """.trimMargin() + val layout = getLayoutWithConstraintLayoutRoot(innerLayout) + lint() + .files(getTestXml(layout)) + .issues(*ALL_ISSUES) + .run() + .expectWarningCount(0) + .expectErrorCount(1) + } + + @Ignore("the lint test API seems to be broken: project0: Error: Unexpected failure during " + + "lint analysis (this is a bug in lint or one of the libraries it depends on). Message: The " + + "prefix \"android\" for attribute \"android:layout_width\" associated with an element type \"TextView\" is not bound.") + @Test + fun `GIVEN a file with a ConstraintLayout and a custom view ConstraintLayout layout WHEN detecting THEN warning`() { + val innerLayout = """<TextView + | android:layout_width="match_parent" + | android:layout_height="match_parent"> + | + | </$FQCN_CUSTOM_VIEW> + """.trimMargin() + lint() + .files(getTestXml(innerLayout)) + .issues(*ALL_ISSUES) + .run() + .expectWarningCount(1) + .expectErrorCount(0) + } + + private fun getTestXml(code: String) = TestFiles.xml("res/layout/activity_home.xml", code) + + private fun getLayoutWithConstraintLayoutRoot(innerLayout: String = "") = """<!-- This Source Code Form is subject to the terms of the Mozilla Public + | - License, v. 2.0. If a copy of the MPL was not distributed with this + | - file, You can obtain one at http://mozilla.org/MPL/2.0/. --> + | <$FQCN_CONSTRAINT_LAYOUT + | xmlns:android="http://schemas.android.com/apk/res/android" + | android:layout_width="match_parent" + | android:layout_height="match_parent"> + | + | $innerLayout + | + | </$FQCN_CONSTRAINT_LAYOUT>""".trimMargin() +} |