Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "SysUI linter: Clean Architecture Dependency Rule." into tm-qpr-dev

Browse Source
This commit is contained in:
Ale Nijamkin
2023-01-18 17:34:52 +00:00
committed by Android (Google) Code Review
parent a3e78cf864 878ae084a4
commit 2f12b73182
3 changed files with 455 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

150
packages/SystemUI/checks/src/com/android/internal/systemui/lint/CleanArchitectureDependencyViolationDetector.kt Normal file
View File

@@ -0,0 +1,150 @@
/*
* Copyright (C) 2023 The Android Open Source Project
*
* 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.android.internal.systemui.lint
import com.android.tools.lint.client.api.UElementHandler
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.Scope
import com.android.tools.lint.detector.api.Severity
import org.jetbrains.uast.UElement
import org.jetbrains.uast.UFile
import org.jetbrains.uast.UImportStatement
/**
* Detects violations of the Dependency Rule of Clean Architecture.
*
* The rule states that code in each layer may only depend on code in the same layer or the layer
* directly "beneath" that layer in the layer diagram.
*
* In System UI, we have three layers; from top to bottom, they are: ui, domain, and data. As a
* convention, was used packages with those names to place code in the appropriate layer. We also
* make an exception and allow for shared models to live under a separate package named "shared" to
* avoid code duplication.
*
* For more information, please see go/sysui-arch.
*/
@Suppress("UnstableApiUsage")
class CleanArchitectureDependencyViolationDetector : Detector(), Detector.UastScanner {
override fun getApplicableUastTypes(): List<Class<out UElement>> {
return listOf(UFile::class.java)
}
override fun createUastHandler(context: JavaContext): UElementHandler {
return object : UElementHandler() {
override fun visitFile(node: UFile) {
// Check which Clean Architecture layer this file belongs to:
matchingLayer(node.packageName)?.let { layer ->
// The file matches with a Clean Architecture layer. Let's check all of its
// imports.
node.imports.forEach { importStatement ->
visitImportStatement(context, layer, importStatement)
}
}
}
}
}
private fun visitImportStatement(
context: JavaContext,
layer: Layer,
importStatement: UImportStatement,
) {
val importText = importStatement.importReference?.asSourceString() ?: return
val importedLayer = matchingLayer(importText) ?: return
// Now check whether the layer of the file may depend on the layer of the import.
if (!layer.mayDependOn(importedLayer)) {
context.report(
issue = ISSUE,
scope = importStatement,
location = context.getLocation(importStatement),
message =
"The ${layer.packageNamePart} layer may not depend on" +
" the ${importedLayer.packageNamePart} layer.",
)
}
}
private fun matchingLayer(packageName: String): Layer? {
val packageNameParts = packageName.split(".").toSet()
return Layer.values()
.filter { layer -> packageNameParts.contains(layer.packageNamePart) }
.takeIf { it.size == 1 }
?.first()
}
private enum class Layer(
val packageNamePart: String,
val canDependOn: Set<Layer>,
) {
SHARED(
packageNamePart = "shared",
canDependOn = emptySet(), // The shared layer may not depend on any other layer.
),
DATA(
packageNamePart = "data",
canDependOn = setOf(SHARED),
),
DOMAIN(
packageNamePart = "domain",
canDependOn = setOf(SHARED, DATA),
),
UI(
packageNamePart = "ui",
canDependOn = setOf(DOMAIN, SHARED),
),
;
fun mayDependOn(otherLayer: Layer): Boolean {
return this == otherLayer || canDependOn.contains(otherLayer)
}
}
companion object {
@JvmStatic
val ISSUE =
Issue.create(
id = "CleanArchitectureDependencyViolation",
briefDescription = "Violation of the Clean Architecture Dependency Rule.",
explanation =
"""
Following the \"Dependency Rule\" from Clean Architecture, every layer of code \
can only depend code in its own layer or code in the layer directly \
\"beneath\" it. Therefore, the UI layer can only depend on the" Domain layer \
and the Domain layer can only depend on the Data layer. We" do make an \
exception to allow shared models to exist and be shared across layers by \
placing them under shared/model, which should be done with care. For more \
information about Clean Architecture in System UI, please see go/sysui-arch. \
NOTE: if your code is not using Clean Architecture, please feel free to ignore \
this warning.
""",
category = Category.CORRECTNESS,
priority = 8,
severity = Severity.WARNING,
implementation =
Implementation(
CleanArchitectureDependencyViolationDetector::class.java,
Scope.JAVA_FILE_SCOPE,
),
)
}
}

16
packages/SystemUI/checks/src/com/android/internal/systemui/lint/SystemUIIssueRegistry.kt
View File

@@ -27,9 +27,11 @@ import com.google.auto.service.AutoService
class SystemUIIssueRegistry : IssueRegistry() {
override val issues: List<Issue>
get() = listOf(
get() =
listOf(
BindServiceOnMainThreadDetector.ISSUE,
BroadcastSentViaContextDetector.ISSUE,
CleanArchitectureDependencyViolationDetector.ISSUE,
SlowUserQueryDetector.ISSUE_SLOW_USER_ID_QUERY,
SlowUserQueryDetector.ISSUE_SLOW_USER_INFO_QUERY,
NonInjectedMainThreadDetector.ISSUE,
@@ -37,7 +39,7 @@ class SystemUIIssueRegistry : IssueRegistry() {
SoftwareBitmapDetector.ISSUE,
NonInjectedServiceDetector.ISSUE,
StaticSettingsProviderDetector.ISSUE
)
)
override val api: Int
get() = CURRENT_API
@@ -45,9 +47,9 @@ class SystemUIIssueRegistry : IssueRegistry() {
get() = 8
override val vendor: Vendor =
Vendor(
vendorName = "Android",
feedbackUrl = "http://b/issues/new?component=78010",
contact = "jernej@google.com"
)
Vendor(
vendorName = "Android",
feedbackUrl = "http://b/issues/new?component=78010",
contact = "jernej@google.com"
)
}

296
packages/SystemUI/checks/tests/com/android/internal/systemui/lint/CleanArchitectureDependencyViolationDetectorTest.kt Normal file
View File

@@ -0,0 +1,296 @@
/*
* Copyright (C) 2023 The Android Open Source Project
*
* 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.android.internal.systemui.lint
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.Ignore
import org.junit.Test
@Suppress("UnstableApiUsage")
@Ignore("b/254533331")
class CleanArchitectureDependencyViolationDetectorTest : SystemUILintDetectorTest() {
override fun getDetector(): Detector {
return CleanArchitectureDependencyViolationDetector()
}
override fun getIssues(): List<Issue> {
return listOf(
CleanArchitectureDependencyViolationDetector.ISSUE,
)
}
@Test
fun `No violations`() {
lint()
.files(
*LEGITIMATE_FILES,
)
.issues(
CleanArchitectureDependencyViolationDetector.ISSUE,
)
.run()
.expectWarningCount(0)
}
@Test
fun `Violation - domain depends on ui`() {
lint()
.files(
*LEGITIMATE_FILES,
TestFiles.kotlin(
"""
package test.domain.interactor
import test.ui.viewmodel.ViewModel
class BadClass(
private val viewModel: ViewModel,
)
""".trimIndent()
)
)
.issues(
CleanArchitectureDependencyViolationDetector.ISSUE,
)
.testModes(TestMode.DEFAULT)
.run()
.expectWarningCount(1)
.expect(
expectedText =
"""
src/test/domain/interactor/BadClass.kt:3: Warning: The domain layer may not depend on the ui layer. [CleanArchitectureDependencyViolation]
import test.ui.viewmodel.ViewModel
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
0 errors, 1 warnings
""",
)
}
@Test
fun `Violation - ui depends on data`() {
lint()
.files(
*LEGITIMATE_FILES,
TestFiles.kotlin(
"""
package test.ui.viewmodel
import test.data.repository.Repository
class BadClass(
private val repository: Repository,
)
""".trimIndent()
)
)
.issues(
CleanArchitectureDependencyViolationDetector.ISSUE,
)
.testModes(TestMode.DEFAULT)
.run()
.expectWarningCount(1)
.expect(
expectedText =
"""
src/test/ui/viewmodel/BadClass.kt:3: Warning: The ui layer may not depend on the data layer. [CleanArchitectureDependencyViolation]
import test.data.repository.Repository
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
0 errors, 1 warnings
""",
)
}
@Test
fun `Violation - shared depends on all other layers`() {
lint()
.files(
*LEGITIMATE_FILES,
TestFiles.kotlin(
"""
package test.shared.model
import test.data.repository.Repository
import test.domain.interactor.Interactor
import test.ui.viewmodel.ViewModel
class BadClass(
private val repository: Repository,
private val interactor: Interactor,
private val viewmodel: ViewModel,
)
""".trimIndent()
)
)
.issues(
CleanArchitectureDependencyViolationDetector.ISSUE,
)
.testModes(TestMode.DEFAULT)
.run()
.expectWarningCount(3)
.expect(
expectedText =
"""
src/test/shared/model/BadClass.kt:3: Warning: The shared layer may not depend on the data layer. [CleanArchitectureDependencyViolation]
import test.data.repository.Repository
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/test/shared/model/BadClass.kt:4: Warning: The shared layer may not depend on the domain layer. [CleanArchitectureDependencyViolation]
import test.domain.interactor.Interactor
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/test/shared/model/BadClass.kt:5: Warning: The shared layer may not depend on the ui layer. [CleanArchitectureDependencyViolation]
import test.ui.viewmodel.ViewModel
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
0 errors, 3 warnings
""",
)
}
@Test
fun `Violation - data depends on domain`() {
lint()
.files(
*LEGITIMATE_FILES,
TestFiles.kotlin(
"""
package test.data.repository
import test.domain.interactor.Interactor
class BadClass(
private val interactor: Interactor,
)
""".trimIndent()
)
)
.issues(
CleanArchitectureDependencyViolationDetector.ISSUE,
)
.testModes(TestMode.DEFAULT)
.run()
.expectWarningCount(1)
.expect(
expectedText =
"""
src/test/data/repository/BadClass.kt:3: Warning: The data layer may not depend on the domain layer. [CleanArchitectureDependencyViolation]
import test.domain.interactor.Interactor
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
0 errors, 1 warnings
""",
)
}
companion object {
private val MODEL_FILE =
TestFiles.kotlin(
"""
package test.shared.model
import test.some.other.thing.SomeOtherThing
data class Model(
private val name: String,
)
""".trimIndent()
)
private val REPOSITORY_FILE =
TestFiles.kotlin(
"""
package test.data.repository
import test.shared.model.Model
import test.some.other.thing.SomeOtherThing
class Repository {
private val models = listOf(
Model("one"),
Model("two"),
Model("three"),
)
fun getModels(): List<Model> {
return models
}
}
""".trimIndent()
)
private val INTERACTOR_FILE =
TestFiles.kotlin(
"""
package test.domain.interactor
import test.data.repository.Repository
import test.shared.model.Model
class Interactor(
private val repository: Repository,
) {
fun getModels(): List<Model> {
return repository.getModels()
}
}
""".trimIndent()
)
private val VIEW_MODEL_FILE =
TestFiles.kotlin(
"""
package test.ui.viewmodel
import test.domain.interactor.Interactor
import test.some.other.thing.SomeOtherThing
class ViewModel(
private val interactor: Interactor,
) {
fun getNames(): List<String> {
return interactor.getModels().map { model -> model.name }
}
}
""".trimIndent()
)
private val NON_CLEAN_ARCHITECTURE_FILE =
TestFiles.kotlin(
"""
package test.some.other.thing
import test.data.repository.Repository
import test.domain.interactor.Interactor
import test.ui.viewmodel.ViewModel
class SomeOtherThing {
init {
val viewModel = ViewModel(
interactor = Interactor(
repository = Repository(),
),
)
}
}
""".trimIndent()
)
private val LEGITIMATE_FILES =
arrayOf(
MODEL_FILE,
REPOSITORY_FILE,
INTERACTOR_FILE,
VIEW_MODEL_FILE,
NON_CLEAN_ARCHITECTURE_FILE,
)
}
}