From 12eccb63ae90af87b633a30c32fa7c6755879585 Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Wed, 24 Apr 2024 12:07:05 +0200 Subject: [PATCH] refactor: Prefer System.currentTimeMillis() to Date().getTime() (#645) Avoids unnecessary object creation. Add a lint rule to catch this. --- .../adapter/ReportNotificationViewHolder.kt | 3 +- .../components/compose/MediaUploader.kt | 3 +- .../SeveredRelationshipsViewHolder.kt | 5 +- .../StatusNotificationViewHolder.kt | 2 +- .../pachli/lint/checks/DateDotTimeDetector.kt | 89 +++++++++++++++++++ .../app/pachli/lint/checks/LintRegistry.kt | 1 + .../lint/checks/DateDotTimeDetectorTest.kt | 50 +++++++++++ 7 files changed, 145 insertions(+), 8 deletions(-) create mode 100644 checks/src/main/java/app/pachli/lint/checks/DateDotTimeDetector.kt create mode 100644 checks/src/test/java/app/pachli/lint/checks/DateDotTimeDetectorTest.kt diff --git a/app/src/main/java/app/pachli/adapter/ReportNotificationViewHolder.kt b/app/src/main/java/app/pachli/adapter/ReportNotificationViewHolder.kt index aef9f1543..12f2a0f05 100644 --- a/app/src/main/java/app/pachli/adapter/ReportNotificationViewHolder.kt +++ b/app/src/main/java/app/pachli/adapter/ReportNotificationViewHolder.kt @@ -34,7 +34,6 @@ import app.pachli.util.StatusDisplayOptions import app.pachli.util.getRelativeTimeSpanString import app.pachli.viewdata.NotificationViewData import at.connyduck.sparkbutton.helpers.Utils -import java.util.Date class ReportNotificationViewHolder( private val binding: ItemReportNotificationBinding, @@ -90,7 +89,7 @@ class ReportNotificationViewHolder( ) binding.notificationSummary.text = itemView.context.getString( R.string.notification_summary_report_format, - getRelativeTimeSpanString(itemView.context, report.createdAt.time, Date().time), + getRelativeTimeSpanString(itemView.context, report.createdAt.time, System.currentTimeMillis()), report.statusIds?.size ?: 0, ) binding.notificationCategory.text = getTranslatedCategory(itemView.context, report.category) diff --git a/app/src/main/java/app/pachli/components/compose/MediaUploader.kt b/app/src/main/java/app/pachli/components/compose/MediaUploader.kt index 2dc36275b..1d08014e5 100644 --- a/app/src/main/java/app/pachli/components/compose/MediaUploader.kt +++ b/app/src/main/java/app/pachli/components/compose/MediaUploader.kt @@ -42,7 +42,6 @@ import java.io.File import java.io.FileInputStream import java.io.FileOutputStream import java.io.IOException -import java.util.Date import javax.inject.Inject import javax.inject.Singleton import kotlinx.coroutines.CoroutineScope @@ -271,7 +270,7 @@ class MediaUploader @Inject constructor( val fileExtension = map.getExtensionFromMimeType(mimeType) val filename = "%s_%s_%s.%s".format( context.getString(R.string.app_name), - Date().time.toString(), + System.currentTimeMillis().toString(), randomAlphanumericString(10), fileExtension, ) diff --git a/app/src/main/java/app/pachli/components/notifications/SeveredRelationshipsViewHolder.kt b/app/src/main/java/app/pachli/components/notifications/SeveredRelationshipsViewHolder.kt index a54fafbcb..f10947c36 100644 --- a/app/src/main/java/app/pachli/components/notifications/SeveredRelationshipsViewHolder.kt +++ b/app/src/main/java/app/pachli/components/notifications/SeveredRelationshipsViewHolder.kt @@ -26,7 +26,6 @@ import app.pachli.databinding.ItemSeveredRelationshipsBinding import app.pachli.util.StatusDisplayOptions import app.pachli.util.getRelativeTimeSpanString import app.pachli.viewdata.NotificationViewData -import java.util.Date class SeveredRelationshipsViewHolder( private val binding: ItemSeveredRelationshipsBinding, @@ -42,7 +41,7 @@ class SeveredRelationshipsViewHolder( HtmlCompat.FROM_HTML_MODE_LEGACY, ) - binding.datetime.text = getRelativeTimeSpanString(itemView.context, event.createdAt.time, Date().time) + binding.datetime.text = getRelativeTimeSpanString(itemView.context, event.createdAt.time, System.currentTimeMillis()) binding.notificationSummary.text = itemView.context.resources.getQuantityString( R.plurals.notification_severed_relationships_summary_report_fmt, @@ -59,7 +58,7 @@ class SeveredRelationshipsViewHolder( binding.notificationCategory.text = itemView.context.getString(resourceId) } else { if (payloads.any { it == StatusBaseViewHolder.Key.KEY_CREATED }) { - binding.datetime.text = getRelativeTimeSpanString(itemView.context, event.createdAt.time, Date().time) + binding.datetime.text = getRelativeTimeSpanString(itemView.context, event.createdAt.time, System.currentTimeMillis()) } } } diff --git a/app/src/main/java/app/pachli/components/notifications/StatusNotificationViewHolder.kt b/app/src/main/java/app/pachli/components/notifications/StatusNotificationViewHolder.kt index 8b82271df..73a2cbc17 100644 --- a/app/src/main/java/app/pachli/components/notifications/StatusNotificationViewHolder.kt +++ b/app/src/main/java/app/pachli/components/notifications/StatusNotificationViewHolder.kt @@ -173,7 +173,7 @@ internal class StatusNotificationViewHolder( val readoutAloud: CharSequence if (createdAt != null) { val then = createdAt.time - val now = Date().time + val now = System.currentTimeMillis() readout = getRelativeTimeSpanString(binding.statusMetaInfo.context, then, now) readoutAloud = DateUtils.getRelativeTimeSpanString( then, diff --git a/checks/src/main/java/app/pachli/lint/checks/DateDotTimeDetector.kt b/checks/src/main/java/app/pachli/lint/checks/DateDotTimeDetector.kt new file mode 100644 index 000000000..d919224e4 --- /dev/null +++ b/checks/src/main/java/app/pachli/lint/checks/DateDotTimeDetector.kt @@ -0,0 +1,89 @@ +/* + * Copyright 2024 Pachli Association + * + * This file is a part of Pachli. + * + * This program is free software; you can redistribute it and/or modify it under the terms of the + * GNU General Public License as published by the Free Software Foundation; either version 3 of the + * License, or (at your option) any later version. + * + * Pachli is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even + * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General + * Public License for more details. + * + * You should have received a copy of the GNU General Public License along with Pachli; if not, + * see . + */ + +package app.pachli.lint.checks + +import com.android.tools.lint.checks.DataFlowAnalyzer +import com.android.tools.lint.detector.api.Category +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.LintFix +import com.android.tools.lint.detector.api.Scope +import com.android.tools.lint.detector.api.Severity +import com.android.tools.lint.detector.api.SourceCodeScanner +import com.intellij.psi.PsiMethod +import org.jetbrains.uast.UCallExpression +import org.jetbrains.uast.UMethod +import org.jetbrains.uast.getParentOfType + +class DateDotTimeDetector : Detector(), SourceCodeScanner { + override fun getApplicableConstructorTypes(): List = listOf(CLASS_DATE) + + val fix = LintFix.create() + .name("Replace with `System.currentTimeMillis()`") + .replace() + .text("Date().time") + .with("System.currentTimeMillis()") + .independent(true) + .build() + + // Look for a Date() constructor call followed by a getTime() method call + override fun visitConstructor(context: JavaContext, node: UCallExpression, constructor: PsiMethod) { + val method = node.getParentOfType(UMethod::class.java) ?: return + val analyzer = object : DataFlowAnalyzer(listOf(node)) { + var reported = false + + override fun receiver(call: UCallExpression) { + if (reported) return + if (call.methodName != METHOD_GET_TIME) return + + reported = true + context.report( + issue = ISSUE, + scope = node, + location = context.getCallLocation(call, includeReceiver = true, includeArguments = true), + message = "Use `System.currentTimeMillis()`", + quickfixData = fix, + ) + } + } + method.accept(analyzer) + } + + companion object { + private const val CLASS_DATE = "java.util.Date" + private const val METHOD_GET_TIME = "getTime" + + val ISSUE = Issue.create( + id = "DateDotTimeDetector", + briefDescription = "Don't use `Date().time`, use `System.currentTimeMillis()`", + explanation = """ + Calling `Date().time` / `Date().getTime()` is unnecessary object creation. Use + `System.currentTimeMillis()` to avoid this. + """, + category = Category.CORRECTNESS, + priority = 6, + severity = Severity.WARNING, + implementation = Implementation( + DateDotTimeDetector::class.java, + Scope.JAVA_FILE_SCOPE, + ), + ) + } +} diff --git a/checks/src/main/java/app/pachli/lint/checks/LintRegistry.kt b/checks/src/main/java/app/pachli/lint/checks/LintRegistry.kt index 74ee383f5..3aeafbfb0 100644 --- a/checks/src/main/java/app/pachli/lint/checks/LintRegistry.kt +++ b/checks/src/main/java/app/pachli/lint/checks/LintRegistry.kt @@ -10,6 +10,7 @@ class LintRegistry : IssueRegistry() { override val issues: List get() = listOf( AndroidxToolbarDetector.ISSUE, + DateDotTimeDetector.ISSUE, IntentDetector.ISSUE, ) diff --git a/checks/src/test/java/app/pachli/lint/checks/DateDotTimeDetectorTest.kt b/checks/src/test/java/app/pachli/lint/checks/DateDotTimeDetectorTest.kt new file mode 100644 index 000000000..4f45261cf --- /dev/null +++ b/checks/src/test/java/app/pachli/lint/checks/DateDotTimeDetectorTest.kt @@ -0,0 +1,50 @@ +/* + * Copyright 2024 Pachli Association + * + * This file is a part of Pachli. + * + * This program is free software; you can redistribute it and/or modify it under the terms of the + * GNU General Public License as published by the Free Software Foundation; either version 3 of the + * License, or (at your option) any later version. + * + * Pachli is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even + * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General + * Public License for more details. + * + * You should have received a copy of the GNU General Public License along with Pachli; if not, + * see . + */ + +package app.pachli.lint.checks + +import com.android.tools.lint.checks.infrastructure.LintDetectorTest +import com.android.tools.lint.checks.infrastructure.TestMode +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Issue + +@Suppress("ktlint:standard:function-naming") +class DateDotTimeDetectorTest : LintDetectorTest() { + override fun getDetector(): Detector = DateDotTimeDetector() + + override fun getIssues(): List = listOf(DateDotTimeDetector.ISSUE) + + fun `test Intent component constructor emits warning`() { + lint().files( + kotlin( + """ + package test.pkg + + import java.util.Date + + fun getTimeMills() = Date().time + """, + + ).indented(), + ).allowMissingSdk().testModes(TestMode.DEFAULT).run().expect( + """src/test/pkg/test.kt:5: Warning: Use System.currentTimeMillis() [DateDotTimeDetector] +fun getTimeMills() = Date().time + ~~~~~~~~~~~ +0 errors, 1 warnings""", + ) + } +}