diff --git a/build.gradle b/build.gradle index cbd863870b37..995bd796fab6 100644 --- a/build.gradle +++ b/build.gradle @@ -535,3 +535,12 @@ subprojects { project -> } } } + +tasks.register("lint-a-c") { + subprojects.each{ + if (it.tasks.findByName("lint") != null && it.projectDir.absolutePath.contains("android-components")) { + dependsOn it.tasks.named("lint") + println("Subproject ${it.name} has a lint task") + } + } +} diff --git a/mobile/android/android-components/android-lint.gradle b/mobile/android/android-components/android-lint.gradle index 42620d42999d..1dc7be246181 100644 --- a/mobile/android/android-components/android-lint.gradle +++ b/mobile/android/android-components/android-lint.gradle @@ -1,7 +1,7 @@ android { lint { warningsAsErrors = true - abortOnError = true + abortOnError = false // With our L10N process its totally possible to have missing or (temporarily) extra translations. disable 'MissingTranslation', @@ -15,5 +15,7 @@ android { // "We do not impose rules on locales" // https://github.com/mozilla-mobile/android-components/pull/11069 'TypographyDashes' + sarifReport true + sarifOutput file("../../../build/reports/lint/lint-report-${project.name}.sarif.json") } } diff --git a/mobile/android/android-components/components/compose/base/src/main/java/mozilla/components/compose/base/modifier/Modifier.kt b/mobile/android/android-components/components/compose/base/src/main/java/mozilla/components/compose/base/modifier/Modifier.kt index 9e9a6712aa4d..14c601f9f1af 100644 --- a/mobile/android/android-components/components/compose/base/src/main/java/mozilla/components/compose/base/modifier/Modifier.kt +++ b/mobile/android/android-components/components/compose/base/src/main/java/mozilla/components/compose/base/modifier/Modifier.kt @@ -10,6 +10,7 @@ import androidx.annotation.FloatRange import androidx.compose.foundation.clickable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableLongStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue @@ -84,7 +85,7 @@ fun Modifier.debouncedClickable( debounceInterval: Long = 1000L, onClick: () -> Unit, ) = composed { - var lastClickTime: Long by remember { mutableStateOf(0) } + var lastClickTime: Long by remember { mutableLongStateOf(0) } this.then( Modifier.clickable( diff --git a/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/internal/SearchUrlBuilder.kt b/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/internal/SearchUrlBuilder.kt index 48f232641290..b8d3b2514058 100644 --- a/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/internal/SearchUrlBuilder.kt +++ b/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/internal/SearchUrlBuilder.kt @@ -79,7 +79,7 @@ private fun paramSubstitution(template: String, query: String, inputEncoding: St } private fun normalize(input: String): String { - val trimmedInput = input.trim { it <= ' ' } + val trimmedInput = input.trim() var uri = trimmedInput.toUri() if (TextUtils.isEmpty(uri.scheme)) { diff --git a/mobile/android/android-components/components/support/utils/src/main/java/mozilla/components/support/ktx/util/URLStringUtils.kt b/mobile/android/android-components/components/support/utils/src/main/java/mozilla/components/support/ktx/util/URLStringUtils.kt index 91f99acb0e9f..a47b90d40c6f 100644 --- a/mobile/android/android-components/components/support/utils/src/main/java/mozilla/components/support/ktx/util/URLStringUtils.kt +++ b/mobile/android/android-components/components/support/utils/src/main/java/mozilla/components/support/ktx/util/URLStringUtils.kt @@ -148,7 +148,7 @@ object URLStringUtils { * Determine whether a string is a valid search query URL. */ fun isValidSearchQueryUrl(url: String): Boolean { - var trimmedUrl = url.trim { it <= ' ' } + var trimmedUrl = url.trim() if (!trimmedUrl.matches("^.+?://.+?".toRegex())) { // UI hint url doesn't have http scheme, so add it if necessary trimmedUrl = "http://$trimmedUrl" diff --git a/mobile/android/fenix/app/build.gradle b/mobile/android/fenix/app/build.gradle index 1ac79cca1c3a..b7dbfe6d1e40 100644 --- a/mobile/android/fenix/app/build.gradle +++ b/mobile/android/fenix/app/build.gradle @@ -248,6 +248,9 @@ android { lint { lintConfig = file("lint.xml") baseline = file("lint-baseline.xml") + sarifReport = true + sarifOutput = file("../build/reports/lint/lint-report.sarif.json") + abortOnError = false } // https://issuetracker.google.com/issues/379732901 diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonDetailsBindingDelegate.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonDetailsBindingDelegate.kt index 3cfcc9575c0f..8ddd7b396229 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonDetailsBindingDelegate.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonDetailsBindingDelegate.kt @@ -30,7 +30,7 @@ import java.util.Locale interface AddonDetailsInteractor { /** - * Open the given URL in the browser. + * Open the given URL in the browser.a */ fun openWebsite(url: Uri) diff --git a/mobile/android/focus-android/app/build.gradle b/mobile/android/focus-android/app/build.gradle index 25f831276dad..40fbd4379ef0 100644 --- a/mobile/android/focus-android/app/build.gradle +++ b/mobile/android/focus-android/app/build.gradle @@ -61,6 +61,9 @@ android { lint { lintConfig = file("lint.xml") baseline = file("lint-baseline.xml") + sarifReport = true + sarifOutput = file("../build/reports/lint/lint-report.sarif.json") + abortOnError = false } // We have a three dimensional build configuration: diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/AddToHomescreenDialogFragment.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/AddToHomescreenDialogFragment.kt index a4ae4517289a..697dc7161528 100644 --- a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/AddToHomescreenDialogFragment.kt +++ b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/AddToHomescreenDialogFragment.kt @@ -93,12 +93,12 @@ class AddToHomescreenDialogFragment : DialogFragment() { requireContext(), IconGenerator.generateLauncherIcon(requireContext(), iconUrl), iconUrl, - editableTitle.text.toString().trim { it <= ' ' }, + editableTitle.text.toString().trim(), blockingEnabled, requestDesktop, ) - val hasEditedTitle = initialTitle != editableTitle.text.toString().trim { it <= ' ' } + val hasEditedTitle = initialTitle != editableTitle.text.toString().trim() AddToHomeScreen.addButtonTapped.record( AddToHomeScreen.AddButtonTappedExtra( hasEditedTitle = hasEditedTitle, diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/UrlInputFragment.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/UrlInputFragment.kt index 82f1f264f568..a901b26c9fd3 100644 --- a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/UrlInputFragment.kt +++ b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/UrlInputFragment.kt @@ -501,7 +501,7 @@ class UrlInputFragment : } internal fun onCommit(input: String) { - if (input.trim { it <= ' ' }.isNotEmpty()) { + if (input.trim().isNotEmpty()) { handleCrashTrigger(input) binding.browserToolbar.hideKeyboard() @@ -612,7 +612,7 @@ class UrlInputFragment : internal fun onTextChange(text: String) { searchSuggestionsViewModel.setSearchQuery(text) - if (text.trim { it <= ' ' }.isEmpty()) { + if (text.trim().isEmpty()) { binding.searchViewContainer.isVisible = false if (!isOverlay) { diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/shortcut/HomeScreen.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/shortcut/HomeScreen.kt index c455e43044ab..d41ba642b98b 100644 --- a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/shortcut/HomeScreen.kt +++ b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/shortcut/HomeScreen.kt @@ -66,7 +66,7 @@ object HomeScreen { blockingEnabled: Boolean, requestDesktop: Boolean, ) { - val shortcutTitle = if (TextUtils.isEmpty(title.trim { it <= ' ' })) { + val shortcutTitle = if (TextUtils.isEmpty(title.trim())) { generateTitleFromUrl(url) } else { title diff --git a/taskcluster/kinds/source-test/mozlint-android.yml b/taskcluster/kinds/source-test/mozlint-android.yml index 8da338545b64..a81d3b23c819 100644 --- a/taskcluster/kinds/source-test/mozlint-android.yml +++ b/taskcluster/kinds/source-test/mozlint-android.yml @@ -3,11 +3,7 @@ # file, You can obtain one at http://mozilla.org/MPL/2.0/. --- -# Just one TC job that runs all the lints. There's no real advantage -# to running them separately, and this way we perform the expensive -# clone and somewhat expensive compilation of the Android -# intermediates just once. -lints: +task-defaults: always-target: true attributes: build_platform: android @@ -16,7 +12,6 @@ lints: description: Android lints platform: lint/opt treeherder: - symbol: A(lints) kind: test tier: 1 worker-type: t-linux-docker @@ -62,6 +57,20 @@ lints: type: directory run: using: run-task + tooltool-downloads: internal # For internal toolchains. + fetches: + toolchain: + # Aliases aren't allowed for toolchains installed by fetch. + - linux64-android-gradle-dependencies + - linux64-android-sdk-linux-repack + - linux64-jdk-repack + - linux64-node + + +lints: + treeherder: + symbol: A(lints) + run: command: > ln -s $MOZ_FETCHES_DIR/android-gradle-dependencies $GECKO_PATH && ln -s $MOZ_FETCHES_DIR/android-sdk-linux $GECKO_PATH && @@ -75,18 +84,7 @@ lints: --linter android-checkstyle --linter android-lint --linter android-test - --linter android-fenix - --linter android-ac - --linter android-focus * - tooltool-downloads: internal # For internal toolchains. - fetches: - toolchain: - # Aliases aren't allowed for toolchains installed by fetch. - - linux64-android-gradle-dependencies - - linux64-android-sdk-linux-repack - - linux64-jdk-repack - - linux64-node when: files-changed: # Source files. @@ -107,3 +105,87 @@ lints: # Other misc lint related files. - 'python/mozlint/**' - 'tools/lint/**' + +fenix: + worker-type: t-linux-xlarge-source + treeherder: + symbol: A(fenix-lints) + run: + command: > + ln -s $MOZ_FETCHES_DIR/android-gradle-dependencies $GECKO_PATH && + ln -s $MOZ_FETCHES_DIR/android-sdk-linux $GECKO_PATH && + ln -s $MOZ_FETCHES_DIR/node $GECKO_PATH && + cd $GECKO_PATH && + ./mach --log-no-times build pre-export export && + ./mach --log-no-times lint -f treeherder -f json:/builds/worker/mozlint.json + --linter android-fenix + * + when: + files-changed: + # Source files. + - 'mobile/android/fenix/**' + # Build stuff. + - 'mobile/android/**/Makefile.in' + - 'mobile/android/config/**' + - 'mobile/android/gradle.configure' + - 'mobile/android/**/moz.build' + - '**/*.gradle' + # Other misc lint related files. + - 'python/mozlint/**' + - 'tools/lint/**' + +focus: + worker-type: t-linux-xlarge-source + treeherder: + symbol: A(focus-lints) + run: + command: > + ln -s $MOZ_FETCHES_DIR/android-gradle-dependencies $GECKO_PATH && + ln -s $MOZ_FETCHES_DIR/android-sdk-linux $GECKO_PATH && + ln -s $MOZ_FETCHES_DIR/node $GECKO_PATH && + cd $GECKO_PATH && + ./mach --log-no-times build pre-export export && + ./mach --log-no-times lint -f treeherder -f json:/builds/worker/mozlint.json + --linter android-focus + * + when: + files-changed: + # Source files. + - 'mobile/android/focus-android/**' + # Build stuff. + - 'mobile/android/**/Makefile.in' + - 'mobile/android/config/**' + - 'mobile/android/gradle.configure' + - 'mobile/android/**/moz.build' + - '**/*.gradle' + # Other misc lint related files. + - 'python/mozlint/**' + - 'tools/lint/**' + +android-components: + worker-type: t-linux-xlarge-source + treeherder: + symbol: A(ac-lints) + run: + command: > + ln -s $MOZ_FETCHES_DIR/android-gradle-dependencies $GECKO_PATH && + ln -s $MOZ_FETCHES_DIR/android-sdk-linux $GECKO_PATH && + ln -s $MOZ_FETCHES_DIR/node $GECKO_PATH && + cd $GECKO_PATH && + ./mach --log-no-times build pre-export export && + ./mach --log-no-times lint -f treeherder -f json:/builds/worker/mozlint.json + --linter android-ac + * + when: + files-changed: + # Source files. + - 'mobile/android/android-components/**' + # Build stuff. + - 'mobile/android/**/Makefile.in' + - 'mobile/android/config/**' + - 'mobile/android/gradle.configure' + - 'mobile/android/**/moz.build' + - '**/*.gradle' + # Other misc lint related files. + - 'python/mozlint/**' + - 'tools/lint/**' diff --git a/tools/lint/android/lints.py b/tools/lint/android/lints.py index ab5c3fd6134c..c3bb5e67c8ce 100644 --- a/tools/lint/android/lints.py +++ b/tools/lint/android/lints.py @@ -162,6 +162,10 @@ def fenix_format(_paths, config, fix=None, **lintargs): config, fix, os.path.join("mobile", "android", "fenix"), + lint_tasks=[ + "fenix:lint", + "fenix:lintFenixDebug", + ], **lintargs, ) @@ -171,6 +175,7 @@ def ac_format(_paths, config, fix=None, **lintargs): config, fix, os.path.join("mobile", "android", "android-components"), + lint_tasks=["lint-a-c"], **lintargs, ) @@ -180,11 +185,12 @@ def focus_format(_paths, config, fix=None, **lintargs): config, fix, os.path.join("mobile", "android", "focus-android"), + lint_tasks=["focus-android:lint"], **lintargs, ) -def report_gradlew(config, fix, subdir, **lintargs): +def report_gradlew(config, fix, subdir, lint_tasks=[], **lintargs): topsrcdir = lintargs["root"] topobjdir = lintargs["topobjdir"] @@ -195,14 +201,13 @@ def report_gradlew(config, fix, subdir, **lintargs): extra_args = lintargs.get("extra_args") or [] - for task in tasks: - gradle( - lintargs["log"], - topsrcdir=topsrcdir, - topobjdir=topobjdir, - tasks=[task], - extra_args=extra_args + ["-p", os.path.join(topsrcdir, subdir)], - ) + gradle( + lintargs["log"], + topsrcdir=topsrcdir, + topobjdir=topobjdir, + tasks=tasks, + extra_args=extra_args + ["-p", os.path.join(topsrcdir, subdir), "--continue"], + ) reports = os.path.join(topsrcdir, subdir, "build", "reports") results = [] @@ -216,16 +221,21 @@ def report_gradlew(config, fix, subdir, **lintargs): elif f.startswith(subdir): excludes.append(f.strip()) - try: - tree = ET.parse( - open( - os.path.join( - reports, - "detekt", - "detekt.xml", - ), - ) + detekt_report = None + if os.path.exists(os.path.join(reports, "detekt", "detekt.xml")): + detekt_report = os.path.join(reports, "detekt", "detekt.xml") + elif os.path.join( + topobjdir, "gradle", "build", subdir, "reports", "detekt", "detekt.xml" + ): + detekt_report = os.path.join( + topobjdir, "gradle", "build", subdir, "reports", "detekt", "detekt.xml" ) + else: + print(f"Could not read detekt report: '{detekt_report}'") + pass + + try: + tree = ET.parse(open(detekt_report)) root = tree.getroot() for file in root.findall("file"): @@ -243,6 +253,7 @@ def report_gradlew(config, fix, subdir, **lintargs): } results.append(result.from_config(config, **err)) except FileNotFoundError: + print(f"Could not read detekt report: '{detekt_report}'") pass ktlint_file = "ktlint.json" @@ -274,9 +285,10 @@ def report_gradlew(config, fix, subdir, **lintargs): } results.append(result.from_config(config, **err)) except FileNotFoundError: + print(f"Could not read ktlint report: `{ktlint_file}`") pass - return results + return results + read_lint_report(config, subdir, tasks=lint_tasks, **lintargs) def is_excluded_file(topsrcdir, excludes, file): @@ -407,6 +419,98 @@ def lint(_paths, config, **lintargs): return results +def read_lint_report(config, subdir, tasks=[], **lintargs): + topsrcdir = lintargs["root"] + topobjdir = lintargs["topobjdir"] + + gradle( + lintargs["log"], + topsrcdir=topsrcdir, + topobjdir=topobjdir, + tasks=tasks, + extra_args=lintargs.get("extra_args") or [], + ) + + reports = os.path.join(topsrcdir, subdir, "build", "reports") + + excludes = [] + for path in EXCLUSION_FILES: + with open(os.path.join(topsrcdir, path)) as fh: + for f in fh.readlines(): + if "*" in f: + excludes.extend(glob.glob(f.strip())) + elif f.startswith(subdir): + excludes.append(f.strip()) + + try: + files = os.listdir( + os.path.join( + reports, + "lint", + ) + ) + + results = [] + for file in files: + data = json.load( + open( + os.path.join(reports, "lint", file), + ) + ).get( + "runs", [{}] + )[0] + + issues = data.get("results", []) + rules = data.get("tool", {}).get("driver", {}).get("rules", []) + + for issue in issues: + dir = os.path.join(topsrcdir, subdir) + if subdir != os.path.join("mobile", "android", "android-components"): + dir = os.path.join(topsrcdir, "mobile", "android") + name = os.path.join( + dir, + issue.get("locations", [{}])[0] + .get("physicalLocation", {}) + .get("artifactLocation", {}) + .get("uri"), + ) + + if is_excluded_file(topsrcdir, excludes, name) and not "/res/" in name: + continue + + level = "error" + if "level" in issue: + level = issue["level"] + elif "ruleIndex" in issue and len(rules) > issue["ruleIndex"]: + rule_level = ( + rules[issue["ruleIndex"]] + .get("defaultConfiguration", {}) + .get("level") + ) + if rule_level: + level = rule_level + + err = { + "rule": issue.get("ruleId"), + "path": name, + "lineno": issue.get("locations", [{}])[0] + .get("physicalLocation", {}) + .get("region", {}) + .get("startLine"), + "column": issue.get("locations", [{}])[0] + .get("physicalLocation", {}) + .get("region", {}) + .get("startColumn"), + "message": issue.get("message", {}).get("text"), + "level": level, + } + results.append(result.from_config(config, **err)) + return results + except FileNotFoundError: + print("Could not read lint report from ", subdir) + return [] + + def _parse_checkstyle_output(config, topsrcdir=None, report_path=None): tree = ET.parse(open(report_path)) root = tree.getroot()