-
-
Notifications
You must be signed in to change notification settings - Fork 360
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Android Linting Example #3931
base: main
Are you sure you want to change the base?
Changes from all commits
43ed8c7
8bae57e
577375f
5cab0a8
92a165e
4e82c26
1d3805a
bd67ec5
6588076
c8f33f1
2c78931
1429e01
d0c7f11
9e21cee
60b1cbb
f6420eb
0994b1a
67deba8
03c0381
bea5080
8c32d89
1be883b
2cae0ea
428e7c0
24d91e0
3d01603
9585600
df95eb3
49d4717
54ded22
b910ada
a6e2fc5
195a51e
e7de9dc
20b4107
b4801fe
7e24224
4133561
44cbf19
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
= Linting Android Projects | ||
:page-aliases: Linting_Android_Projects.adoc | ||
|
||
include::partial$gtag-config.adoc[] | ||
|
||
This page covers essential practices for maintaining and enforcing code quality | ||
in Android projects using the Mill build tool. Proper linting helps detect | ||
and resolve potential issues early, promoting better performance, security, | ||
and user experience. | ||
|
||
|
||
include::partial$example/android/javalib/3-linting.adoc[] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
<manifest xmlns:android="http://schemas.android.com/apk/res/android" package="com.helloworld.app" android:versionCode="1" android:versionName="1.0"> | ||
<uses-sdk android:minSdkVersion="9" android:targetSdkVersion="35"/> | ||
<application android:label="@string/app_name" android:theme="@android:style/Theme.Light.NoTitleBar" android:debuggable="true"> | ||
<activity android:name=".MainActivity" | ||
android:exported="true"> | ||
<intent-filter> | ||
<action android:name="android.intent.action.MAIN"/> | ||
<category android:name="android.intent.category.LAUNCHER"/> | ||
</intent-filter> | ||
</activity> | ||
</application> | ||
</manifest> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<lint> | ||
<issue id="MissingApplicationIcon" severity="ignore" /> | ||
</lint> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
<resources> | ||
<color name="white">#FFFFFF</color> | ||
<color name="text_green">#34A853</color> | ||
</resources> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
<resources> | ||
<string name="app_name">HelloWorldApp</string> | ||
<string name="hello_world">Hello, World Java!</string> | ||
</resources> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
package com.helloworld.app; | ||
|
||
import android.app.Activity; | ||
import android.os.Bundle; | ||
import android.view.Gravity; | ||
import android.view.ViewGroup.LayoutParams; | ||
import android.widget.TextView; | ||
|
||
public class MainActivity extends Activity { | ||
@Override | ||
protected void onCreate(Bundle savedInstanceState) { | ||
super.onCreate(savedInstanceState); | ||
|
||
// Create a new TextView | ||
TextView textView = new TextView(this); | ||
|
||
// Set the text to the string resource | ||
textView.setText(getString(R.string.hello_world)); | ||
|
||
// Set text size | ||
textView.setTextSize(32); | ||
|
||
// Center the text within the view | ||
textView.setGravity(Gravity.CENTER); | ||
|
||
// Set the layout parameters (width and height) | ||
textView.setLayoutParams( | ||
new LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT)); | ||
|
||
// Set the text color using a resource | ||
textView.setTextColor(getResources().getColor(R.color.text_green)); | ||
|
||
// Set the background color using a resource | ||
textView.setBackgroundColor(getResources().getColor(R.color.white)); | ||
|
||
// Set the content view to display the TextView | ||
setContentView(textView); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,143 @@ | ||||||
// == Linting with Basic Config | ||||||
|
||||||
// This Mill build configuration includes a linting step, which is essential for ensuring code | ||||||
// quality and adhering to best practices in Android development projects. Running the `androidLintRun` task | ||||||
// produces a detailed HTML report by default, identifying potential issues in the code, such as performance, | ||||||
// security, and usability warnings. This helps maintain the health and quality of the codebase. | ||||||
|
||||||
// In this example, we set a custom config to generate the report file in txt format and not HTML. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually HTML is generated as well |
||||||
|
||||||
//// SNIPPET:BUILD | ||||||
package build | ||||||
|
||||||
import mill._ | ||||||
import mill.javalib.android.{AndroidSdkModule, AndroidAppModule, AndroidLintReportFormat} | ||||||
|
||||||
// Create and configure an Android SDK module to manage Android SDK paths and tools. | ||||||
object androidSdkModule0 extends AndroidSdkModule { | ||||||
def buildToolsVersion = "35.0.0" | ||||||
def bundleToolVersion = "1.17.2" | ||||||
} | ||||||
|
||||||
// Actual android application with linting config | ||||||
object app extends AndroidAppModule { | ||||||
def androidSdkModule = mill.define.ModuleRef(androidSdkModule0) | ||||||
|
||||||
// Set path to the custom `lint.xml` config file. It is usually at the root of the project | ||||||
def androidLintConfigPath = Task { Some(PathRef(millSourcePath / "lint.xml")) } | ||||||
|
||||||
// Set path to the custom `baseline.xml` file. It is usually at the root of the project | ||||||
def androidLintBaselinePath = Task { Some(PathRef(millSourcePath / "lint-baseline.xml")) } | ||||||
|
||||||
// Set the linting report to be generated as both text and html files | ||||||
def androidLintReportFmt = Task { Seq(AndroidLintReportFormat.Txt, AndroidLintReportFormat.Html) } | ||||||
|
||||||
// Set to true (default), stops the build if errors are found | ||||||
def androidLintAbortOnError = false | ||||||
} | ||||||
|
||||||
/** See Also: app/AndroidManifest.xml */ | ||||||
|
||||||
// The `AndroidManifest.xml` file shown above is flawed with hardcoded string linting problem so it can be spoted by running the `androidLintRun` task. | ||||||
|
||||||
/** See Also: app/resources/values/colors.xml */ | ||||||
/** See Also: app/resources/values/strings.xml */ | ||||||
/** See Also: app/src/main/java/com/helloworld/app/MainActivity.java */ | ||||||
/** See Also: app/lint.xml */ | ||||||
|
||||||
// The `lint.xml` file shown above ignores the `MissingApplicationIcon` error in the code as | ||||||
// the project is for demostration. There are no icons for the demo project. | ||||||
|
||||||
////SNIPPET:END | ||||||
|
||||||
/** Usage | ||||||
|
||||||
> ./mill app.androidLintRun | ||||||
|
||||||
> ls out/app/androidLintRun.dest # Display full path to the linting report in text file | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: it is not only txt file, but also html |
||||||
report.html | ||||||
report.txt | ||||||
|
||||||
> ls app/ # List out all files in the app directory to check lint-baseline file exists | ||||||
AndroidManifest.xml | ||||||
lint-baseline.xml | ||||||
lint.xml | ||||||
resources | ||||||
src | ||||||
|
||||||
> cat out/app/androidLintRun.dest/report.txt # Display content of the linting report | ||||||
AndroidManifest.xml:3: Error: Avoid hardcoding the debug mode; leaving it out allows debug and release builds to automatically assign one [HardcodedDebugMode] | ||||||
|
||||||
> sed -i.bak 's/ android:debuggable="true"//g' app/AndroidManifest.xml # Fix the HardcodedDebugMode warning issue from `AndroidManifest.xml` | ||||||
|
||||||
> ./mill app.androidLintRun # Rerun it for new changes to reflect | ||||||
|
||||||
> cat out/app/androidLintRun.dest/report.txt # Check the content of report again | ||||||
lint-baseline.xml: Information: 1 errors/warnings were listed in the baseline file (lint-baseline.xml) but not found in the project; perhaps they have been fixed? Unmatched issue types: HardcodedDebugMode [LintBaseline] | ||||||
|
||||||
*/ | ||||||
|
||||||
// == Linting with Custom Configurations | ||||||
|
||||||
// The commands below are demos on using `androidLintRun` task with some custom configuratins on a sample project. | ||||||
// The demos are adjusting severity lelvel of linting issue, enable and disable linting issue | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
// and lastly suppressing linting issue in both XML and Java files. Some output of the changes are shown below. | ||||||
|
||||||
/** Usage | ||||||
|
||||||
> sed -i.bak 's/severity="ignore"/severity="warning"/g' app/lint.xml # Set `MissingApplicationIcon` severity level to warning | ||||||
c0d33ngr marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
> ./mill app.androidLintRun # Rerun it for new changes to reflect | ||||||
|
||||||
> cat out/app/androidLintRun.dest/report.txt # Output the changes in the report | ||||||
AndroidManifest.xml:3: Warning: Should explicitly set android:icon, there is no default [MissingApplicationIcon] | ||||||
|
||||||
> sed -i.bak 's/severity="warning"/severity="ignore"/g' app/lint.xml # Revert the severity level of `MissingApplicationIcon` | ||||||
|
||||||
> sed -i '$s|.*|<!-- Unused resource -->\n<string name="unused_string">This string is unused</string>\n</resources>|' app/resources/values/strings.xml # Add UnusedResources issue | ||||||
|
||||||
> ./mill app.androidLintRun # Rerun it for the linting tool to detect the UnusedResources | ||||||
|
||||||
> cat out/app/androidLintRun.dest/report.txt # Output the changes in the report | ||||||
resources/values/strings.xml:5: Warning: The resource R.string.unused_string appears to be unused [UnusedResources] | ||||||
|
||||||
> sed -i 's|<resources|<resources xmlns:tools="http://schemas.android.com/tools" tools:ignore="UnusedResources"|' app/resources/values/strings.xml # Suppress linting check for UnusedResources issue | ||||||
|
||||||
> ./mill app.androidLintRun # Rerun it for new changes to reflect | ||||||
|
||||||
> cat out/app/androidLintRun.dest/report.txt # Output the report content | ||||||
0 errors, 0 warnings | ||||||
|
||||||
> sed -i.bak '/<lint>/a \ <issue id="SyntheticAccessor" severity="error" />' app/lint.xml # Enable SyntheticAccessor issue cause it's disabled by default | ||||||
|
||||||
> cat app/lint.xml | ||||||
<?xml version="1.0" encoding="utf-8"?> | ||||||
<lint> | ||||||
<issue id="SyntheticAccessor" severity="error" /> | ||||||
<issue id="MissingApplicationIcon" severity="ignore" /> | ||||||
</lint> | ||||||
|
||||||
> sed -i.bak '$a\\nclass SyntheticAccessorViolation {\n private String privateField = "access me, if you dare";\n\n static class Inner {\n static void usePrivateField(SyntheticAccessorViolation instance) {\n System.out.println(instance.privateField);\n }\n }\n}' app/src/main/java/com/helloworld/app/MainActivity.java # Add SyntheticAccessor issue to code | ||||||
|
||||||
> ./mill app.androidLintRun # Rerun it for new changes to reflect | ||||||
|
||||||
> cat out/app/androidLintRun.dest/report.txt # Output the report content | ||||||
src/main/java/com/helloworld/app/MainActivity.java:46: Error: Access to private field privateField of class SyntheticAccessorViolation requires synthetic accessor [SyntheticAccessor] | ||||||
|
||||||
> sed -i.bak -e '/import android.annotation.SuppressLint;/d' -e '/class SyntheticAccessorViolation {/,/^}/d' app/src/main/java/com/helloworld/app/MainActivity.java # Add SuppressLint import | ||||||
|
||||||
> sed -i.bak '/class SyntheticAccessorViolation {/i @SuppressLint("SyntheticAccessor")' app/src/main/java/com/helloworld/app/MainActivity.java # Add the SuppressLint annotation | ||||||
|
||||||
> ./mill app.androidLintRun # Rerun it for new changes to reflect | ||||||
|
||||||
> cat out/app/androidLintRun.dest/report.txt # Output the report content | ||||||
0 errors, 0 warnings | ||||||
|
||||||
> sed -i.bak -e '/import android.annotation.SuppressLint;/d' -e '/class SyntheticAccessorViolation {/,/^}/d' app/src/main/java/com/helloworld/app/MainActivity.java # Remove the SyntheticAccessor issue from code | ||||||
|
||||||
> ./mill app.androidLintRun # Rerun it for new changes to reflect | ||||||
|
||||||
> cat out/app/androidLintRun.dest/report.txt # Output the report content | ||||||
0 errors, 0 warnings | ||||||
|
||||||
*/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,32 @@ import mill.scalalib._ | |
import mill.api.PathRef | ||
import mill.define.ModuleRef | ||
|
||
import upickle.default._ | ||
|
||
/** | ||
* Enumeration for Android Lint report formats, providing predefined formats | ||
* with corresponding flags and file extensions. Includes utility methods | ||
* for implicit conversions, serialization, and retrieving all supported formats. | ||
*/ | ||
object AndroidLintReportFormat extends Enumeration { | ||
protected case class Format(flag: String, extension: String) extends super.Val { | ||
override def toString: String = extension | ||
} | ||
|
||
implicit def valueToFormat(v: Value): Format = v.asInstanceOf[Format] | ||
|
||
val Html: Format = Format("--html", "html") | ||
val Xml: Format = Format("--xml", "xml") | ||
val Txt: Format = Format("--text", "txt") | ||
val Sarif: Format = Format("--sarif", "sarif") | ||
|
||
// Define an implicit ReadWriter for the Format case class | ||
implicit val formatRW: ReadWriter[Format] = macroRW | ||
|
||
// Optional: Add a method to retrieve all possible values | ||
val allFormats: List[Format] = List(Html, Xml, Txt, Sarif) | ||
} | ||
|
||
/** | ||
* Trait for building Android applications using the Mill build tool. | ||
* | ||
|
@@ -62,6 +88,34 @@ trait AndroidAppModule extends JavaModule { | |
*/ | ||
def androidReleaseKeyPath: T[PathRef] = Task.Source(millSourcePath / androidReleaseKeyName()) | ||
|
||
/** | ||
* Specifies the file format(s) of the lint report. Available file formats are defined in AndroidLintReportFormat, | ||
* such as AndroidLintReportFormat.Html, AndroidLintReportFormat.Xml, AndroidLintReportFormat.Txt, and AndroidLintReportFormat.Sarif. | ||
*/ | ||
def androidLintReportFmt: T[Seq[AndroidLintReportFormat.Value]] = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if |
||
Task { Seq(AndroidLintReportFormat.Html) } | ||
|
||
/** | ||
* Specifies the lint configuration XML file path. This allows setting custom lint rules or modifying existing ones. | ||
*/ | ||
def androidLintConfigPath: T[Option[PathRef]] = Task { None } | ||
|
||
/** | ||
* Specifies the lint baseline XML file path. This allows using a baseline to suppress known lint warnings. | ||
*/ | ||
def androidLintBaselinePath: T[Option[PathRef]] = Task { None } | ||
|
||
/** | ||
* Determines whether the build should fail if Android Lint detects any issues. | ||
*/ | ||
def androidLintAbortOnError: Boolean = true | ||
|
||
/** | ||
* Specifies additional arguments for the Android Lint tool. | ||
* Allows for complete customization of the lint command. | ||
*/ | ||
def androidLintArgs: T[Seq[String]] = Task { Seq.empty[String] } | ||
|
||
/** | ||
* Extracts JAR files and resources from AAR dependencies. | ||
* | ||
|
@@ -339,4 +393,63 @@ trait AndroidAppModule extends JavaModule { | |
|
||
PathRef(keystoreFile) | ||
} | ||
|
||
/** | ||
* Runs the Android Lint tool to generate a report on code quality issues. | ||
* | ||
* This method utilizes Android Lint, a tool provided by the Android SDK, | ||
* to analyze the source code for potential bugs, performance issues, and | ||
* best practices compliance. It generates a report in the specified format. | ||
* | ||
* The report is saved in the task's destination directory as "report.html" by | ||
* default. It can be changed to other file format such as xml, txt and sarif. | ||
* | ||
* For more details on the Android Lint tool, refer to: | ||
* [[https://developer.android.com/studio/write/lint]] | ||
*/ | ||
def androidLintRun() = Task.Command { | ||
|
||
val formats = androidLintReportFmt() | ||
|
||
// Generate the alternating flag and file path strings | ||
val reportArg: Seq[String] = formats.toSeq.flatMap { format => | ||
Seq(format.flag, (Task.dest / s"report.${format.extension}").toString) | ||
} | ||
|
||
// Set path to generated `.jar` files and/or `.class` files | ||
val cp = runClasspath().map(_.path).filter(os.exists).mkString(":") | ||
|
||
// Set path to the location of the project source codes | ||
val src = sources().map(_.path / "main" / "java").filter(os.exists).mkString(":") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you shouldn't hardcode path as Ideally, in order to not have a huge path with each individual files there, it should be a way to find a common roots for the given set of the files, but I don't know if there are utility methods in the project's codebase to compute that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I needed directory path to the sources file that's why I did that using the most common There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actual files can be under any path of |
||
|
||
// Set path to the location of the project ressource codes | ||
val res = resources().map(_.path).filter(os.exists).mkString(":") | ||
|
||
// Prepare the lint configuration argument if the config path is set | ||
val configArg = androidLintConfigPath().map(config => | ||
Seq("--config", config.path.toString) | ||
).getOrElse(Seq.empty) | ||
|
||
// Prepare the lint baseline argument if the baseline path is set | ||
val baselineArg = androidLintBaselinePath().map(baseline => | ||
Seq("--baseline", baseline.path.toString) | ||
).getOrElse(Seq.empty) | ||
|
||
os.call( | ||
Seq( | ||
androidSdkModule().lintToolPath().path.toString, | ||
millSourcePath.toString, | ||
"--classpath", | ||
cp, | ||
"--sources", | ||
src, | ||
"--resources", | ||
res | ||
) ++ configArg ++ baselineArg ++ reportArg ++ androidLintArgs(), | ||
check = androidLintAbortOnError | ||
) | ||
|
||
PathRef(Task.dest) | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably a change to rollback? seems like this was a separator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please I do not fully understand what you meant here