summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTor Norbye <tnorbye@google.com>2021-04-20 16:14:27 -0700
committerTor Norbye <tnorbye@google.com>2021-05-03 15:24:47 +0000
commit6eb3246c557aabe3062acf2bf7795b2c00086989 (patch)
tree1bf977d9ae28f202b9f10f5be637ea11910c41c3
parent65a750b98f70eaf1cc7d1cce46fbf83e47a849e7 (diff)
downloadbase-6eb3246c557aabe3062acf2bf7795b2c00086989.tar.gz
Reuse LintFixPerformer from LintFixVerifier
There was a lot of code duplication between LintFixPerformer (used from the Gradle lintFix task etc) and the LintFixVerifier (used from lint unit tests to verify quickfix results). They have different purposes and behave differently (for example, the unit test will write "descriptions" of things to do, such as showing a URL, as well as adding in markup to show things like selection and caret positions, and the fix task will limit fixes to those marked as safe to apply non-interactively etc. However, for the editing-based fixes (string replacement, attribute set/clear, and annotate), the code was to some extent duplicated, but the unit testing code not as accurate, and as a result it did not handle composite fixes correctly. This CL migrates the LintFixVerifier to reuse the LintFixPerformer for the editing operations, extends that one with some additional abilities, and removes the old code from the LintFixVerifier. It also updates a few unit tests to reflect the new correct state. This also revealed some bugs which needed to be fixed, such as correctly handling updating XML attributes that have already been set. It also updates the lint fix verifier to handle fixes which update multiple files; all the diffs are now listed in the diff report. One of the tests was updated to check this. And finally, it makes it possible to create new files from quick fixes. Test: Existing + a new regression test and some existing tests extended Bug: 186140311 Change-Id: Iae5cd9c12c0dec267ca27260bc675ff622ae51e8
-rw-r--r--lint/cli/src/main/java/com/android/tools/lint/LintFixPerformer.kt323
-rw-r--r--lint/cli/src/main/java/com/android/tools/lint/SarifReporter.kt2
-rw-r--r--lint/cli/src/main/java/com/android/tools/lint/XmlReader.kt46
-rw-r--r--lint/cli/src/main/java/com/android/tools/lint/XmlWriter.kt30
-rw-r--r--lint/docs/api-guide/changes.md.html34
-rw-r--r--lint/libs/lint-api/src/main/java/com/android/tools/lint/client/api/LintDriver.kt6
-rw-r--r--lint/libs/lint-api/src/main/java/com/android/tools/lint/detector/api/LintFix.kt260
-rw-r--r--lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/FontDetector.java2
-rw-r--r--lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/GradleDetector.kt6
-rw-r--r--lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/ManifestResourceDetector.java9
-rw-r--r--lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/RtlDetector.java6
-rw-r--r--lint/libs/lint-model/src/main/java/com/android/tools/lint/model/PathVariables.kt13
-rw-r--r--lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/LintFixVerifier.kt466
-rw-r--r--lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintClient.java114
-rw-r--r--lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintResult.kt4
-rw-r--r--lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintTask.java13
-rw-r--r--lint/libs/lint-tests/src/test/java/com/android/tools/lint/LintFixPerformerTest.kt2
-rw-r--r--lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/ButtonDetectorTest.java8
-rw-r--r--lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/FontDetectorTest.java12
-rw-r--r--lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/IgnoreWithoutReasonDetectorTest.kt2
-rw-r--r--lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/infrastructure/LintFixVerifierTest.kt193
-rw-r--r--lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/infrastructure/TestLintResultTest.kt (renamed from lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintResultTest.kt)0
22 files changed, 1081 insertions, 470 deletions
diff --git a/lint/cli/src/main/java/com/android/tools/lint/LintFixPerformer.kt b/lint/cli/src/main/java/com/android/tools/lint/LintFixPerformer.kt
index c18c5bb571..3672572704 100644
--- a/lint/cli/src/main/java/com/android/tools/lint/LintFixPerformer.kt
+++ b/lint/cli/src/main/java/com/android/tools/lint/LintFixPerformer.kt
@@ -31,11 +31,12 @@ import com.android.SdkConstants.XMLNS_PREFIX
import com.android.tools.lint.detector.api.Incident
import com.android.tools.lint.detector.api.LintFix
import com.android.tools.lint.detector.api.LintFix.AnnotateFix
+import com.android.tools.lint.detector.api.LintFix.CreateFileFix
import com.android.tools.lint.detector.api.LintFix.GroupType
import com.android.tools.lint.detector.api.LintFix.LintFixGroup
import com.android.tools.lint.detector.api.LintFix.ReplaceString
-import com.android.tools.lint.detector.api.LintFix.ReplaceString.INSERT_BEGINNING
-import com.android.tools.lint.detector.api.LintFix.ReplaceString.INSERT_END
+import com.android.tools.lint.detector.api.LintFix.ReplaceString.Companion.INSERT_BEGINNING
+import com.android.tools.lint.detector.api.LintFix.ReplaceString.Companion.INSERT_END
import com.android.tools.lint.detector.api.LintFix.SetAttribute
import com.android.tools.lint.detector.api.Location
import com.android.tools.lint.detector.api.Severity
@@ -52,18 +53,33 @@ import java.io.IOException
import java.io.PrintWriter
import java.util.regex.Pattern
import javax.xml.parsers.ParserConfigurationException
+import kotlin.math.abs
+import kotlin.math.max
+import kotlin.math.min
/** Support for applying quickfixes directly. */
open class LintFixPerformer constructor(
val client: LintCliClient,
- private val printStatistics: Boolean = true
+ /**
+ * Whether to emit statistics about number of files modified and
+ * number of edits applied
+ */
+ private val printStatistics: Boolean = true,
+ /**
+ * Should applied fixes be limited to those marked as safe to be
+ * applied automatically?
+ */
+ private val requireAutoFixable: Boolean = true,
+ /**
+ * Should we include markers in the applied files like indicators
+ * for the marker and selection?
+ */
+ private val includeMarkers: Boolean = false
) {
private fun getFileData(
fileMap: MutableMap<File, PendingEditFile>,
- incident: Incident
+ file: File
): PendingEditFile {
- val location = getLocation(incident)
- val file = location.file
return fileMap[file] ?: run {
val source = client.getSourceText(file)
val fileData = PendingEditFile(client, file, source.toString())
@@ -77,8 +93,9 @@ open class LintFixPerformer constructor(
incident: Incident,
lintFix: LintFix
) {
- val fileData = getFileData(fileMap, incident)
- if (addEdits(fileData, getLocation(incident), lintFix)) {
+ val location = getLocation(incident, lintFix)
+ val fileData = getFileData(fileMap, location.file)
+ if (addEdits(fileData, location, lintFix)) {
incident.wasAutoFixed = true
}
}
@@ -88,6 +105,16 @@ open class LintFixPerformer constructor(
return applyEdits(files)
}
+ fun fix(incident: Incident, fixes: List<LintFix>): Boolean {
+ val fileMap = mutableMapOf<File, PendingEditFile>()
+ for (fix in fixes) {
+ if (canAutoFix(fix, requireAutoFixable)) {
+ registerFix(fileMap, incident, fix)
+ }
+ }
+ return applyEdits(fileMap.values.toList())
+ }
+
@VisibleForTesting
fun fix(
file: File,
@@ -95,7 +122,7 @@ open class LintFixPerformer constructor(
text: String = file.readText(Charsets.UTF_8)
): Boolean {
val pendingEditFile = PendingEditFile(client, file, text)
- fixes.filter { canAutoFix(it) }.forEach {
+ fixes.filter { canAutoFix(it, requireAutoFixable) }.forEach {
addEdits(pendingEditFile, null, it)
}
return applyEdits(listOf(pendingEditFile))
@@ -107,6 +134,15 @@ open class LintFixPerformer constructor(
val editMap: MutableMap<String, Int> = mutableMapOf()
for (fileData in files) {
+ val newText = fileData.createText
+ if (newText != null) {
+ writeFile(fileData, addMarkers(fileData, newText))
+ } else if (fileData.createBytes != null) {
+ writeFile(fileData, fileData.createBytes)
+ } else if (fileData.delete) {
+ writeFile(fileData, null)
+ continue
+ }
// Sort fixes in descending order from location
if (fileData.edits.isEmpty()) {
continue
@@ -134,8 +170,25 @@ open class LintFixPerformer constructor(
editMap[key] = count + 1
fileContents = edit.apply(fileContents)
+
+ // Move selection offsets accordingly. These aren't just
+ // inserted as their own edits since they may apply to text
+ // that hasn't been applied yet (e.g. when using a regex
+ // pattern from a starting offset to identify what is to be selected)
+ fileData.selection?.let {
+ val dot = min(it.dot, it.mark)
+ val mark = max(it.dot, it.mark)
+ if (dot != Integer.MIN_VALUE) {
+ it.dot = edit.adjustOffset(dot, true)
+ if (mark != Integer.MIN_VALUE) {
+ it.mark = edit.adjustOffset(mark, false)
+ }
+ }
+ }
}
+ fileContents = addMarkers(fileData, fileContents)
+
writeFile(fileData, fileContents)
editedFileCount++
}
@@ -147,6 +200,49 @@ open class LintFixPerformer constructor(
return editedFileCount > 0
}
+ /**
+ * Indicates caret position with a `|` and the selection range using
+ * square brackets if set by the fix
+ */
+ private fun addMarkers(
+ fileData: PendingEditFile,
+ fileContents: String
+ ): String {
+ if (includeMarkers) {
+ fileData.selection?.let {
+ val (pattern, offset, mark) = it
+ val selectStart: Int
+ val selectEnd: Int
+ val matcher = pattern?.matcher(fileContents)
+ if (matcher != null && matcher.find(offset)) {
+ if (matcher.groupCount() > 0) {
+ selectStart = matcher.start(1)
+ selectEnd = matcher.end(1)
+ } else {
+ selectStart = matcher.start()
+ selectEnd = matcher.end()
+ }
+ } else if (pattern == null && offset != Integer.MIN_VALUE) {
+ val effectiveMark = if (mark != Integer.MIN_VALUE) mark else offset
+ selectStart = min(offset, effectiveMark)
+ selectEnd = max(offset, effectiveMark)
+ } else {
+ return fileContents
+ }
+ return if (selectStart == selectEnd) {
+ fileContents.substring(0, selectStart) + "|" + fileContents.substring(selectEnd)
+ } else {
+ fileContents.substring(0, selectStart) +
+ "[" +
+ fileContents.substring(selectStart, selectEnd) +
+ "]|" +
+ fileContents.substring(selectEnd)
+ }
+ }
+ }
+ return fileContents
+ }
+
protected open fun printStatistics(
writer: PrintWriter,
editMap: MutableMap<String, Int>,
@@ -164,10 +260,22 @@ open class LintFixPerformer constructor(
}
protected open fun writeFile(
- file: PendingEditFile,
+ pendingFile: PendingEditFile,
contents: String
) {
- file.file.writeText(contents, Charsets.UTF_8)
+ writeFile(pendingFile, contents.toByteArray(Charsets.UTF_8))
+ }
+
+ protected open fun writeFile(
+ pendingFile: PendingEditFile,
+ contents: ByteArray?
+ ) {
+ if (contents == null) {
+ pendingFile.file.delete()
+ } else {
+ pendingFile.file.parentFile?.mkdirs()
+ pendingFile.file.writeBytes(contents)
+ }
}
private fun findApplicableFixes(incidents: List<Incident>): List<PendingEditFile> {
@@ -179,7 +287,7 @@ open class LintFixPerformer constructor(
// separated out again in applyFix
var all = true
for (sub in data.fixes) {
- if (!canAutoFix(sub)) {
+ if (!canAutoFix(sub, requireAutoFixable)) {
all = false
break
}
@@ -192,7 +300,7 @@ open class LintFixPerformer constructor(
}
// else: for GroupType.ALTERNATIVES, we don't auto fix: user must pick
// which one to apply.
- } else if (canAutoFix(data)) {
+ } else if (canAutoFix(data, requireAutoFixable)) {
registerFix(fileMap, incident, data)
}
}
@@ -211,9 +319,10 @@ open class LintFixPerformer constructor(
client.log(
Severity.WARNING, null,
"Overlapping edits in quickfixes; skipping. " +
- "Involved fixes: ${prev.fix.displayName} in [" +
+ "Involved fixes: ${prev.fix.getDisplayName()} in [" +
"${prev.startOffset}-${prev.endOffset}] and ${
- fix.fix.displayName} in [${fix.startOffset}-${fix.endOffset}]"
+ fix.fix.getDisplayName()
+ } in [${fix.startOffset}-${fix.endOffset}]"
)
return true
}
@@ -233,6 +342,8 @@ open class LintFixPerformer constructor(
addSetAttribute(file, lintFix, location)
} else if (lintFix is AnnotateFix) {
addAnnotation(file, lintFix, location)
+ } else if (lintFix is CreateFileFix) {
+ addCreateFile(file, lintFix)
} else if (lintFix is LintFixGroup && lintFix.type == GroupType.COMPOSITE) {
var all = true
for (nested in lintFix.fixes) {
@@ -257,6 +368,20 @@ open class LintFixPerformer constructor(
return addReplaceString(file, replaceFix, fixLocation)
}
+ private fun addCreateFile(
+ file: PendingEditFile,
+ fix: CreateFileFix
+ ): Boolean {
+ if (fix.delete) {
+ file.delete = true
+ return true
+ }
+ file.createText = fix.text
+ file.createBytes = fix.binary
+ fix.selectPattern?.let { file.selection = Selection(Pattern.compile(it), 0) }
+ return true
+ }
+
private fun addSetAttribute(
file: PendingEditFile,
setFix: SetAttribute,
@@ -290,18 +415,21 @@ open class LintFixPerformer constructor(
contents
)
}
+
val element = node as Element
val value = setFix.value
val namespace = setFix.namespace
+ var attributeName = setFix.attribute
+
+ val attr =
+ if (namespace != null) {
+ element.getAttributeNodeNS(namespace, attributeName)
+ } else {
+ element.getAttributeNode(attributeName)
+ }
+
if (value == null) {
// Delete attribute
- val attr =
- if (namespace != null) {
- element.getAttributeNodeNS(namespace, setFix.attribute)
- } else {
- element.getAttributeNode(setFix.attribute)
- }
-
if (attr != null) {
val position = PositionXmlParser.getPosition(attr)
val startOffset: Int = position.startOffset
@@ -315,7 +443,18 @@ open class LintFixPerformer constructor(
}
return false
} else {
- var attributeName = setFix.attribute
+ if (attr != null) {
+ // Already set; change it
+ val position = PositionXmlParser.getPosition(attr)
+ val startOffset: Int = position.startOffset
+ val endOffset: Int = position.endOffset
+ val prefix = attr.name + "=\""
+ val replacement = prefix + XmlUtils.toXmlAttributeValue(value) + "\""
+ file.edits.add(PendingEdit(setFix, contents, startOffset, endOffset, replacement))
+ addValueSelection(value, setFix.dot, setFix.mark, startOffset, prefix.length, file)
+ return true
+ }
+
val insertOffset = findAttributeInsertionOffset(
file.initialText,
element,
@@ -364,16 +503,40 @@ open class LintFixPerformer constructor(
val padLeft = if (!contents[insertOffset - 1].isWhitespace()) " " else ""
val padRight =
if (contents[insertOffset] != '/' && contents[insertOffset] != '>') " " else ""
+
+ val leftPart = "$padLeft$attributeName=\""
+ val valuePart = XmlUtils.toXmlAttributeValue(value)
+ val rightPart = "\"$padRight"
file.edits.add(
PendingEdit(
setFix, contents, insertOffset, insertOffset,
- "$padLeft$attributeName=\"${XmlUtils.toXmlAttributeValue(value)}\"$padRight"
+ leftPart + valuePart + rightPart
)
)
+ addValueSelection(value, setFix.dot, setFix.mark, insertOffset, leftPart.length, file)
return true
}
}
+ private fun addValueSelection(
+ value: String,
+ dot: Int,
+ mark: Int,
+ startOffset: Int,
+ valueOffset: Int,
+ file: PendingEditFile
+ ) {
+ if (dot != Integer.MIN_VALUE) {
+ val valueStart = startOffset + valueOffset
+ val valueDot = valueStart + dot
+ val valueMark = if (mark != Integer.MIN_VALUE) valueStart + mark else valueDot
+ val selectionStart = min(valueDot, valueMark)
+ val selectedString = value.substring(min(dot, mark), max(dot, mark))
+ val pattern = Pattern.compile(Pattern.quote(selectedString))
+ file.selection = Selection(pattern, selectionStart)
+ }
+ }
+
private fun findAttributeInsertionOffset(
xml: String,
element: Element,
@@ -538,12 +701,59 @@ open class LintFixPerformer constructor(
}
}
+ if (includeMarkers && replaceFix.shortenNames) {
+ // This isn't fully shortening names, it's only removing fully qualified names
+ // for symbols already imported.
+ //
+ // Also, this will not correctly handle some conflicts. This is only used for
+ // unit testing lint fixes, not for actually operating on code; for that we're using
+ // IntelliJ's built in import cleanup when running in the IDE.
+ val imported: MutableSet<String> = HashSet()
+ for (line in contents.split("\n").toTypedArray()) {
+ if (line.startsWith("package ")) {
+ var to = line.indexOf(';')
+ if (to == -1) {
+ to = line.length
+ }
+ imported.add(line.substring("package ".length, to).trim { it <= ' ' } + ".")
+ } else if (line.startsWith("import ")) {
+ var from = "import ".length
+ if (line.startsWith("static ")) {
+ from += " static ".length
+ }
+ var to = line.indexOf(';')
+ if (to == -1) {
+ to = line.length
+ }
+ if (line[to - 1] == '*') {
+ to--
+ }
+ imported.add(line.substring(from, to).trim { it <= ' ' })
+ }
+ }
+ for (full in imported) {
+ var clz = full
+ if (replacement.contains(clz)) {
+ if (!clz.endsWith(".")) {
+ val index = clz.lastIndexOf('.')
+ if (index == -1) {
+ continue
+ }
+ clz = clz.substring(0, index + 1)
+ }
+ replacement = replacement.replace(clz, "")
+ }
+ }
+ }
+
+ replaceFix.selectPattern?.let { file.selection = Selection(Pattern.compile(it), start.offset) }
+
val edit = PendingEdit(replaceFix, contents, startOffset, endOffset, replacement)
file.edits.add(edit)
return true
}
- fun computeEdits(incident: Incident, lintFix: LintFix): List<PendingEditFile>? {
+ fun computeEdits(incident: Incident, lintFix: LintFix): List<PendingEditFile> {
val fileMap = mutableMapOf<File, PendingEditFile>()
registerFix(fileMap, incident, lintFix)
return fileMap.values.toList()
@@ -552,6 +762,10 @@ open class LintFixPerformer constructor(
companion object {
fun getLocation(incident: Incident): Location {
val fix = incident.fix
+ return getLocation(incident, fix)
+ }
+
+ fun getLocation(incident: Incident, fix: LintFix?): Location {
if (fix is ReplaceString) {
val range = fix.range
if (range != null) {
@@ -562,15 +776,34 @@ open class LintFixPerformer constructor(
if (range != null) {
return range
}
+ } else if (fix is AnnotateFix) {
+ val range = fix.range
+ if (range != null) {
+ return range
+ }
+ } else if (fix is CreateFileFix) {
+ return Location.create(fix.file)
}
return incident.location
}
+ fun isEditingFix(fix: LintFix): Boolean {
+ return fix is ReplaceString || fix is AnnotateFix || fix is SetAttribute || fix is CreateFileFix
+ }
+
/**
* Not all fixes are eligible for auto-fix; this function checks
* whether a given fix is.
*/
fun canAutoFix(lintFix: LintFix): Boolean {
+ return canAutoFix(lintFix, true)
+ }
+
+ fun canAutoFix(lintFix: LintFix, requireAutoFixable: Boolean): Boolean {
+ if (!requireAutoFixable && !(lintFix is LintFixGroup && lintFix.type == GroupType.ALTERNATIVES)) {
+ return true
+ }
+
if (lintFix is LintFixGroup) {
when (lintFix.type) {
GroupType.ALTERNATIVES ->
@@ -579,7 +812,7 @@ open class LintFixPerformer constructor(
GroupType.COMPOSITE -> {
// All nested fixes must be auto-fixable
for (nested in lintFix.fixes) {
- if (!canAutoFix(nested)) {
+ if (!canAutoFix(nested, requireAutoFixable)) {
return false
}
}
@@ -684,8 +917,14 @@ open class LintFixPerformer constructor(
}
}
+ data class Selection(val pattern: Pattern?, var dot: Int = Integer.MIN_VALUE, var mark: Int = Integer.MIN_VALUE)
+
class PendingEditFile(val client: LintCliClient, val file: File, val initialText: String) {
val edits: MutableList<PendingEdit> = mutableListOf()
+ var selection: Selection? = null
+ var delete: Boolean = false
+ var createText: String? = null
+ var createBytes: ByteArray? = null
private var document: Document? = null
@@ -743,23 +982,37 @@ open class LintFixPerformer constructor(
return contents.substring(0, startOffset) + replacement + contents.substring(endOffset)
}
+ fun adjustOffset(offset: Int, biasLeft: Boolean): Int {
+ return if (offset < startOffset || offset == startOffset && biasLeft) {
+ offset
+ } else {
+ offset - abs(startOffset - endOffset) + replacement.length
+ }
+ }
+
private fun isDelete(): Boolean = endOffset > startOffset
fun fixName(): String {
- return fix.familyName ?: return fix.displayName ?: fix.javaClass.simpleName
+ return fix.getFamilyName() ?: return fix.getDisplayName() ?: fix.javaClass.simpleName
}
override fun toString(): String {
return when {
- replacement.isEmpty() -> "At $startOffset, delete \"${source.substring(
- startOffset,
- endOffset
- )}\""
+ replacement.isEmpty() ->
+ "At $startOffset, delete \"${
+ source.substring(
+ startOffset,
+ endOffset
+ )
+ }\""
startOffset == endOffset -> "At $startOffset, insert \"$replacement\""
- else -> "At $startOffset, change \"${source.substring(
- startOffset,
- endOffset
- )}\" to \"$replacement\""
+ else ->
+ "At $startOffset, change \"${
+ source.substring(
+ startOffset,
+ endOffset
+ )
+ }\" to \"$replacement\""
}
}
diff --git a/lint/cli/src/main/java/com/android/tools/lint/SarifReporter.kt b/lint/cli/src/main/java/com/android/tools/lint/SarifReporter.kt
index 986fec3a7b..9ad3359d8d 100644
--- a/lint/cli/src/main/java/com/android/tools/lint/SarifReporter.kt
+++ b/lint/cli/src/main/java/com/android/tools/lint/SarifReporter.kt
@@ -642,7 +642,7 @@ constructor(client: LintCliClient, output: File) : Reporter(client, output) {
// actually figure out what to change
if (files != null && files.isNotEmpty() && files.any { it.edits.isNotEmpty() }) {
var indent = indent
- val description = fix.displayName ?: "Fix"
+ val description = fix.getDisplayName() ?: "Fix"
writer.indent(indent++).write("{\n")
writer.writeDescription(indent, "description", description, comma = true)
writer.indent(indent++).write("\"artifactChanges\": [\n")
diff --git a/lint/cli/src/main/java/com/android/tools/lint/XmlReader.kt b/lint/cli/src/main/java/com/android/tools/lint/XmlReader.kt
index b247c9b9be..1f3d186841 100644
--- a/lint/cli/src/main/java/com/android/tools/lint/XmlReader.kt
+++ b/lint/cli/src/main/java/com/android/tools/lint/XmlReader.kt
@@ -55,6 +55,7 @@ import java.io.File
import java.io.IOException
import java.util.ArrayDeque
import java.util.ArrayList
+import java.util.Base64
import java.util.HashMap
/** The [XmlReader] can restore the state saved by [XmlWriter] */
@@ -212,6 +213,7 @@ class XmlReader(
TAG_ANNOTATE -> addFix(readFixAnnotate())
TAG_FIX_REPLACE -> addFix(readFixReplace())
TAG_FIX_ATTRIBUTE -> addFix(readFixSetAttribute())
+ TAG_CREATE_FILE -> addFix(readCreateFile())
TAG_FIX_ALTERNATIVES,
TAG_FIX_COMPOSITE -> readFixComposite()
TAG_ISSUES, TAG_INCIDENTS -> {
@@ -311,6 +313,50 @@ class XmlReader(
fixLists.addLast(fixList)
}
+ private fun readCreateFile(): LintFix {
+ var text: String? = null
+ var binary: ByteArray? = null
+ var selectPattern: String? = null
+ var reformat = false
+ var displayName: String? = null
+ var familyName: String? = null
+ var robot = false
+ var independent = false
+ var file: File? = null
+ var delete = false
+
+ val n = parser.attributeCount
+ for (i in 0 until n) {
+ val name = parser.getAttributeName(i)
+ val value = parser.getAttributeValue(i)
+ when (name) {
+ ATTR_FILE -> file = getFile(value)
+ ATTR_DELETE -> delete = value == VALUE_TRUE
+ ATTR_REPLACEMENT -> text = value
+ ATTR_BINARY -> binary = Base64.getDecoder().decode(value)
+ ATTR_SELECT_PATTERN -> selectPattern = value
+ ATTR_REFORMAT -> reformat = true
+ ATTR_DESCRIPTION -> displayName = value
+ ATTR_FAMILY -> familyName = value
+ ATTR_INDEPENDENT -> independent = true
+ ATTR_ROBOT -> robot = true
+ else -> error("Unexpected fix attribute: $name")
+ }
+ }
+
+ val builder = LintFix.create().name(displayName).sharedName(familyName)
+ val fix = when {
+ delete -> builder.deleteFile(file!!)
+ text != null -> builder.newFile(file!!, text)
+ else -> builder.newFile(file!!, binary!!)
+ }
+ return fix
+ .select(selectPattern)
+ .reformat(reformat)
+ .autoFix(robot, independent)
+ .build()
+ }
+
private fun readFixSetAttribute(): LintFix {
var namespace: String? = null
var attributeName: String? = null
diff --git a/lint/cli/src/main/java/com/android/tools/lint/XmlWriter.kt b/lint/cli/src/main/java/com/android/tools/lint/XmlWriter.kt
index aa5c417c8e..654aa17ff6 100644
--- a/lint/cli/src/main/java/com/android/tools/lint/XmlWriter.kt
+++ b/lint/cli/src/main/java/com/android/tools/lint/XmlWriter.kt
@@ -58,6 +58,7 @@ import java.io.BufferedWriter
import java.io.File
import java.io.IOException
import java.io.Writer
+import java.util.Base64
import java.util.Locale
import kotlin.math.max
import kotlin.math.min
@@ -459,7 +460,7 @@ open class XmlWriter constructor(
indent(2)
writer.write("<")
writer.write(TAG_FIX)
- lintFix.displayName?.let {
+ lintFix.getDisplayName()?.let {
writeAttribute(writer, 3, ATTR_DESCRIPTION, it)
}
writeAttribute(
@@ -633,6 +634,27 @@ open class XmlWriter constructor(
writer.write("/>\n")
}
}
+ is LintFix.CreateFileFix -> {
+ indent(indent)
+ writer.write("<")
+ writer.write(TAG_CREATE_FILE)
+ emitFixSharedAttributes(lintFix, indented)
+ val neutralPath = getPath(lintFix.file, incident.project)
+ writeAttribute(writer, indent + 1, ATTR_FILE, neutralPath)
+ if (lintFix.delete) {
+ writeAttribute(writer, indented, ATTR_DELETE, VALUE_TRUE)
+ }
+ lintFix.selectPattern?.let {
+ writeAttribute(writer, indented, ATTR_SELECT_PATTERN, it)
+ }
+ lintFix.text?.let {
+ writeAttribute(writer, indented, ATTR_REPLACEMENT, it)
+ }
+ lintFix.binary?.let {
+ writeAttribute(writer, indented, ATTR_BINARY, Base64.getEncoder().encodeToString(it))
+ }
+ writer.write("/>\n")
+ }
is LintFix.DataMap -> {
indent(indent)
writer.write("<")
@@ -668,10 +690,10 @@ open class XmlWriter constructor(
}
private fun emitFixSharedAttributes(lintFix: LintFix, indent: Int) {
- lintFix.displayName?.let {
+ lintFix.getDisplayName()?.let {
writeAttribute(writer, indent, ATTR_DESCRIPTION, it)
}
- lintFix.familyName?.let {
+ lintFix.getFamilyName()?.let {
writeAttribute(writer, indent, ATTR_FAMILY, it)
}
@@ -774,6 +796,7 @@ const val TAG_MAP = "map"
const val TAG_ENTRY = "entry"
const val TAG_SHOW_URL = "show-url"
const val TAG_ANNOTATE = "annotate"
+const val TAG_CREATE_FILE = "create-file"
const val ATTR_SEVERITY = "severity"
const val ATTR_INT = "int"
const val ATTR_BOOLEAN = "boolean"
@@ -805,6 +828,7 @@ const val ATTR_OLD_STRING = "oldString"
const val ATTR_OLD_PATTERN = "oldPattern"
const val ATTR_SELECT_PATTERN = "selectPattern"
const val ATTR_REPLACEMENT = "replacement"
+const val ATTR_BINARY = "binary"
const val ATTR_SHORTEN_NAMES = "shortenNames"
const val ATTR_REFORMAT = "reformat"
const val ATTR_OFFSET = "offset"
diff --git a/lint/docs/api-guide/changes.md.html b/lint/docs/api-guide/changes.md.html
index 0491444191..a93e3cce2a 100644
--- a/lint/docs/api-guide/changes.md.html
+++ b/lint/docs/api-guide/changes.md.html
@@ -67,24 +67,24 @@ Guide.
warnings or errors.
* There are additional modifier lookup methods for Kotlin modifiers
- on `JavaEvaluator, like isReified(), isCompanion(), isTailRec(), and
- so on.
+ on `JavaEvaluator`, like `isReified()`, `isCompanion()`,
+ `isTailRec()`, and so on.
* API documentation is now available.
-* Certain Kotlin PSI elements have new implementations known as
- _ultra light classes_. Ultra light classes improve performance
- by answering PSI queries "directly from source" rather than
- delegating to the Kotlin compiler backend. You may see ultra
- light classes when accessing the `UElement.javaPsi` property of a
- Kotlin UAST element. They can also appear when resolving references.
- For example, resolving a Kotlin field reference to its declaration
- may result in an instance of `KtUltraLightFieldForSourceDeclaration`.
- As a reminder, Kotlin light classes represent the "Java view" of an
+* Certain Kotlin PSI elements have new implementations known as _ultra
+ light classes_. Ultra light classes improve performance by answering
+ PSI queries "directly from source" rather than delegating to the
+ Kotlin compiler backend. You may see ultra light classes when
+ accessing the `UElement.javaPsi` property of a Kotlin UAST element.
+ They can also appear when resolving references. For example,
+ resolving a Kotlin field reference to its declaration may result in
+ an instance of `KtUltraLightFieldForSourceDeclaration`. As a
+ reminder, Kotlin light classes represent the "Java view" of an
underlying Kotlin PSI element. To access the underlying Kotlin PSI
- element you should use `UElement.sourcePsi` (preferred)
- or otherwise the extension property `PsiElement.unwrapped` (declared
- in `org.jetbrains.kotlin.asJava`).
+ element you should use `UElement.sourcePsi` (preferred) or otherwise
+ the extension property `PsiElement.unwrapped` (declared in
+ `org.jetbrains.kotlin.asJava`).
* There is a new bug where calling `getNameIdentifier()` on Kotlin
fields may return `null`
@@ -95,4 +95,10 @@ Guide.
`visitMethodCall()` callback _and_ the `visitReference()` callback.
Previously only `visitMethodCall()` was triggered.
+* Quickfixes can now create and delete new files; see
+ `LintFix#newFile` and `LintFix#deleteFile`..
+
+* For quickfixes, the `independent` property had inverted logic;
+ this has now been reversed to follow the meaning of the name.
+
<!-- Markdeep: --><style class="fallback">body{visibility:hidden;white-space:pre;font-family:monospace}</style><script src="markdeep.min.js" charset="utf-8"></script><script src="https://morgan3d.github.io/markdeep/latest/markdeep.min.js" charset="utf-8"></script><script>window.alreadyProcessedMarkdeep||(document.body.style.visibility="visible")</script>
diff --git a/lint/libs/lint-api/src/main/java/com/android/tools/lint/client/api/LintDriver.kt b/lint/libs/lint-api/src/main/java/com/android/tools/lint/client/api/LintDriver.kt
index bbb58b55c1..f9a35d35eb 100644
--- a/lint/libs/lint-api/src/main/java/com/android/tools/lint/client/api/LintDriver.kt
+++ b/lint/libs/lint-api/src/main/java/com/android/tools/lint/client/api/LintDriver.kt
@@ -1581,12 +1581,12 @@ class LintDriver(
val classEntries: List<ClassEntry>
classEntries = if (classFolders.isEmpty()) {
// This should be a lint error only if there are source files
- val hasSourceFiles: Boolean = project.javaSourceFolders.any { folder -> folder.walk().any { it.isFile} }
+ val hasSourceFiles: Boolean = project.javaSourceFolders.any { folder -> folder.walk().any { it.isFile } }
if (hasSourceFiles) {
val message = String.format(
"No `.class` files were found in project \"%1\$s\", " +
- "so none of the classfile based checks could be run. " +
- "Does the project need to be built first?",
+ "so none of the classfile based checks could be run. " +
+ "Does the project need to be built first?",
project.name
)
LintClient.report(
diff --git a/lint/libs/lint-api/src/main/java/com/android/tools/lint/detector/api/LintFix.kt b/lint/libs/lint-api/src/main/java/com/android/tools/lint/detector/api/LintFix.kt
index c5e60ac368..9b65fc386b 100644
--- a/lint/libs/lint-api/src/main/java/com/android/tools/lint/detector/api/LintFix.kt
+++ b/lint/libs/lint-api/src/main/java/com/android/tools/lint/detector/api/LintFix.kt
@@ -152,7 +152,7 @@ open class LintFix protected constructor(
/**
* Sets display name and family name. If not supplied a default
- * will be created based on the the type of quickfix.
+ * will be created based on the type of quickfix.
*
* @param displayName the displayName
* @param familyName the "family" name; the shared name to use
@@ -167,7 +167,7 @@ open class LintFix protected constructor(
/**
* Sets display name and family name. If not supplied a default
- * will be created based on the the type of quickfix.
+ * will be created based on the type of quickfix.
*
* @param displayName the displayName
* @param useAsFamilyNameToo if true, use the display name as
@@ -295,6 +295,21 @@ open class LintFix protected constructor(
return ReplaceStringBuilder(displayName, familyName)
}
+ /** Creates a new text file with the given contents */
+ fun newFile(file: File, contents: String): CreateFileBuilder {
+ return CreateFileBuilder(displayName, familyName).file(file).contents(contents)
+ }
+
+ /** Creates a new binary file with the given contents */
+ fun newFile(file: File, contents: ByteArray): CreateFileBuilder {
+ return CreateFileBuilder(displayName, familyName).file(file).contents(contents)
+ }
+
+ /** Deletes the given file or directory */
+ fun deleteFile(file: File): CreateFileBuilder {
+ return CreateFileBuilder(displayName, familyName).delete(file)
+ }
+
/**
* Set or clear an attribute
*
@@ -528,7 +543,7 @@ open class LintFix protected constructor(
/**
* Sets display name and family name. If not supplied a default
- * will be created based on the the type of quickfix.
+ * will be created based on the type of quickfix.
*
* @param displayName the displayName
* @param familyName the "family" name; the shared name to use
@@ -590,7 +605,8 @@ open class LintFix protected constructor(
private var robot = false
private var independent = false
- @RegExp private var oldPattern: String? = null
+ @RegExp
+ private var oldPattern: String? = null
private var range: Location? = null
/**
@@ -607,7 +623,7 @@ open class LintFix protected constructor(
/**
* Sets display name and family name. If not supplied a default
- * will be created based on the the type of quickfix.
+ * will be created based on the type of quickfix.
*
* @param displayName the displayName
* @param familyName the "family" name; the shared name to use
@@ -788,7 +804,7 @@ open class LintFix protected constructor(
* @return this
*/
fun independent(independent: Boolean): ReplaceStringBuilder {
- this.independent = !independent
+ this.independent = independent
return this
}
@@ -833,7 +849,187 @@ open class LintFix protected constructor(
reformat,
range,
robot,
- !independent
+ independent
+ )
+ }
+ }
+
+ /**
+ * A builder for creating (or "un-creating", e.g. deleting) a file
+ */
+ class CreateFileBuilder internal constructor(
+ @field:Nls private var displayName: String?,
+ @field:Nls private var familyName: String?
+ ) {
+ private var selectPattern: String? = null
+ private var delete: Boolean = false
+ private var file: File? = null
+ private var binary: ByteArray? = null
+ private var text: String? = null
+ private var reformat = false
+ private var robot = false
+ private var independent = false
+
+ /**
+ * Sets display name and family name. If not supplied a default
+ * will be created based on the type of quickfix.
+ *
+ * @param displayName the displayName
+ * @param familyName the "family" name; the shared name to use
+ * to apply *all* fixes of the same family name in a single go.
+ * @return this
+ */
+ fun name(displayName: String? = null, familyName: String? = null): CreateFileBuilder {
+ this.displayName = displayName
+ this.familyName = familyName
+ return this
+ }
+
+ /** Sets the file to be created or deleted */
+ fun file(file: File): CreateFileBuilder {
+ assert(this.file == null)
+ this.file = file
+ return this
+ }
+
+ /** Marks the file for deletion */
+ fun delete(file: File): CreateFileBuilder {
+ assert(this.file == null && this.binary == null && this.text == null)
+ this.file = file
+ delete = true
+ return this
+ }
+
+ /** Sets the text contents to be written to the file */
+ fun contents(contents: String): CreateFileBuilder {
+ assert(this.binary == null && !delete)
+ this.text = contents
+ return this
+ }
+
+ /** Sets the binary contents to be written to the file */
+ fun contents(contents: ByteArray): CreateFileBuilder {
+ assert(this.text == null && !delete)
+ this.binary = contents
+ return this
+ }
+
+ /**
+ * Sets a pattern to select; if it contains parentheses,
+ * group(1) will be selected. To just set the caret, use an
+ * empty group.
+ */
+ fun select(@RegExp selectPattern: String?): CreateFileBuilder {
+ this.selectPattern = selectPattern
+ return this
+ }
+
+ /** Whether the newly created file should be reformatted */
+ fun reformat(reformat: Boolean): CreateFileBuilder {
+ this.reformat = reformat
+ return this
+ }
+
+ /**
+ * Sets whether this fix can be applied by a robot, e.g. does
+ * not require human intervention. These kinds of fixes can be
+ * automatically applied when lint is run in fix-mode where it
+ * applies all the suggested (eligible) fixes.
+ *
+ * Examples of fixes which are not auto-fixable:
+ * 1. A fix which introduces a semantic change that may not be
+ * desirable. For example, lint may warn that the use
+ * of an API is discouraged and offer a similar but not
+ * identical replacement; in this case the developer
+ * needs to consider the implications of the suggestion.
+ * 2. A fix for a problem where just a part of the solution is
+ * offered as a fix, and there are many other plausible
+ * paths a developer might take, such as lint telling
+ * you that you have too many actions in the toolbar,
+ * and a fix is offered to move each action into a menu.
+ *
+ * @param robot whether this fix can be applied by a robot, e.g.
+ * does not require human intervention
+ * @return this
+ */
+ fun robot(robot: Boolean): CreateFileBuilder {
+ this.robot = robot
+ return this
+ }
+
+ /**
+ * Whether this fix is independent of other fixes getting
+ * applied.
+ *
+ * Lint can automatically apply all fixes which are independent
+ * in a single pass. An example of an independent fix is removal
+ * of an unused import; removing one unused import does not
+ * invalidate a warning (and fix) for another unused import. (Of
+ * course, it's possible that another fix will introduce a new
+ * dependency on the formerly unused class, but this is rare.)
+ *
+ * However, if we have a duplicate declaration warning, we might
+ * put a fix on each one of the duplicates to delete them; if we
+ * apply one, we wouldn't want to apply the other. In fix mode,
+ * lint will only apply the first fix in a compilation unit
+ * that is not independent; it will then need to re-analyze the
+ * compilation unit a second time, and if there are additional
+ * fixes found, apply just the first such dependent fix, and so
+ * on. This means that for N fixes that are not independent, it
+ * will reanalyze the file N times, which is obviously slower.
+ *
+ * @param independent whether it is **not** the case that
+ * applying other fixes simultaneously can invalidate this fix
+ * @return this
+ */
+ fun independent(independent: Boolean): CreateFileBuilder {
+ this.independent = independent
+ return this
+ }
+
+ /**
+ * Sets options related to auto-applying this fix. Convenience
+ * method for setting both [robot] and [independent]
+ *
+ * @param robot whether this fix can be applied by a robot, e.g.
+ * does not require human intervention
+ * @param independent whether it is **not** the case that
+ * applying other fixes simultaneously can invalidate this fix
+ * @return this
+ */
+ fun autoFix(robot: Boolean, independent: Boolean): CreateFileBuilder {
+ robot(robot)
+ independent(independent)
+ return this
+ }
+
+ /**
+ * Convenience method for [autoFix]: indicates that this fix can
+ * safely be applied in auto-fix mode, in parallel with other
+ * fixes.
+ *
+ * @return this
+ */
+ fun autoFix(): CreateFileBuilder {
+ autoFix(robot = true, independent = true)
+ return this
+ }
+
+ /**
+ * Constructs a [LintFix] for this file creation or deletion fix
+ */
+ fun build(): LintFix {
+ return CreateFileFix(
+ displayName,
+ familyName,
+ selectPattern,
+ delete,
+ file!!,
+ binary,
+ text,
+ reformat,
+ robot,
+ independent
)
}
}
@@ -882,7 +1078,7 @@ open class LintFix protected constructor(
/**
* Sets display name and family name. If not supplied a default
- * will be created based on the the type of quickfix.
+ * will be created based on the type of quickfix.
*
* @param displayName the displayName
* @param familyName the "family" name; the shared name to use
@@ -1501,7 +1697,12 @@ open class LintFix protected constructor(
"Delete \"$oldString\""
} else "Delete"
}
- "Replace with $replacement"
+ var preview = replacement
+ val lineIndex = preview.indexOf('\n')
+ if (lineIndex != -1) {
+ preview = preview.substring(0, lineIndex) + "..."
+ }
+ "Replace with $preview"
}
}
@@ -1574,6 +1775,42 @@ open class LintFix protected constructor(
}
}
+ /**
+ * Fix descriptor for creating or deleting a file. This class/API is
+ * **only** intended for IDE use. Lint checks should be accessing
+ * the builder class instead - [create].
+ */
+ class CreateFileFix(
+ displayName: String?,
+ familyName: String?,
+ /**
+ * Pattern to select; if it contains parentheses, group(1) will
+ * be selected
+ */
+ val selectPattern: String?,
+ val delete: Boolean,
+ val file: File,
+ val binary: ByteArray?,
+ val text: String?,
+ val reformat: Boolean,
+ robot: Boolean,
+ independent: Boolean
+ ) : LintFix(displayName, familyName) {
+ init {
+ this.robot = robot
+ this.independent = independent
+ }
+
+ override fun getDisplayName(): String {
+ return super.getDisplayName()
+ ?: return if (delete) {
+ "Delete ${file.name}"
+ } else {
+ "Create ${file.name}"
+ }
+ }
+ }
+
companion object {
/**
* Marker inserted in various places to indicate that something
@@ -1662,7 +1899,7 @@ open class LintFix protected constructor(
* on to the starting and ending offsets, to help reduce active
* memory usage in the IDE; see b/151240516
*/
- private fun extractOffsets(range: Location): Location? {
+ private fun extractOffsets(range: Location): Location {
val start = range.start
val end = range.end
return if (start != null && end != null) {
@@ -1672,7 +1909,8 @@ open class LintFix protected constructor(
DefaultPosition(-1, -1, end.offset)
)
} else {
- null
+ val pos = DefaultPosition(-1, -1, 0)
+ create(range.file, pos, pos)
}
}
}
diff --git a/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/FontDetector.java b/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/FontDetector.java
index 99349cd817..a388f62a7a 100644
--- a/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/FontDetector.java
+++ b/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/FontDetector.java
@@ -364,7 +364,7 @@ public class FontDetector extends ResourceXmlDetector {
provider = mFontLoader.findOnlyKnownProvider();
}
- LintFix.GroupBuilder fix = fix().composite();
+ LintFix.GroupBuilder fix = fix().composite().name("Set missing attributes");
for (String missingAttribute : missingAttributes) {
String value = generateNewValue(missingAttribute, provider);
if (value == null) {
diff --git a/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/GradleDetector.kt b/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/GradleDetector.kt
index c7bcb7e236..c720869916 100644
--- a/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/GradleDetector.kt
+++ b/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/GradleDetector.kt
@@ -634,7 +634,7 @@ open class GradleDetector : Detector(), GradleScanner {
.replace()
.text(configuration)
.with(api)
- .independent(true)
+ .autoFix()
.build()
val implementationFix = fix()
.name("Replace '$configuration' with '$implementation'")
@@ -642,7 +642,7 @@ open class GradleDetector : Detector(), GradleScanner {
.replace()
.text(configuration)
.with(implementation)
- .independent(true)
+ .autoFix()
.build()
val fixes = fix()
@@ -694,7 +694,7 @@ open class GradleDetector : Detector(), GradleScanner {
.replace()
.text(configuration)
.with(replacement)
- .independent(true)
+ .autoFix()
.build()
val message = "Add annotation processor to processor path using `$replacement`" +
" instead of `$configuration`"
diff --git a/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/ManifestResourceDetector.java b/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/ManifestResourceDetector.java
index 9f053872eb..954aadd59e 100644
--- a/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/ManifestResourceDetector.java
+++ b/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/ManifestResourceDetector.java
@@ -169,12 +169,13 @@ public class ManifestResourceDetector extends ResourceXmlDetector {
Collections.sort(list);
String message = getErrorMessage(Joiner.on(", ").join(list));
Location location = context.getValueLocation(attribute);
- context.report(ISSUE, attribute, location, message);
+
// Secondary locations?
// Not relevant when running in the IDE and analyzing a single
// file; no point highlighting matches in other files (which
// will be cleared when you visit them.
if (!context.getDriver().isIsolated()) {
+ Location curr = location;
for (ResourceItem item : items) {
if (!list.contains(item.getConfiguration().getQualifierString())) {
continue;
@@ -183,11 +184,13 @@ public class ManifestResourceDetector extends ResourceXmlDetector {
client.getXmlParser().getValueLocation(client, item);
if (secondary != null) {
secondary.setMessage("This value will not be used");
- location.setSecondary(secondary);
- location = secondary;
+ curr.setSecondary(secondary);
+ curr = secondary;
}
}
}
+
+ context.report(ISSUE, attribute, location, message);
}
}
}
diff --git a/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/RtlDetector.java b/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/RtlDetector.java
index 5084cc71ee..fd0731ead2 100644
--- a/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/RtlDetector.java
+++ b/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/RtlDetector.java
@@ -590,6 +590,9 @@ public class RtlDetector extends LayoutDetector implements SourceCodeScanner {
+ "`gravity` attributes: was `%1$s`, expected `%2$s`",
gravitySpec, expectedGravity);
Location location = context.getValueLocation(attribute);
+ Location secondary = context.getValueLocation(gravityNode);
+ secondary.setMessage("Incompatible direction here");
+ location.setSecondary(secondary);
reportRtl(
context,
COMPAT,
@@ -598,9 +601,6 @@ public class RtlDetector extends LayoutDetector implements SourceCodeScanner {
message,
null,
APPLIES_REQUIRES_RTL);
- Location secondary = context.getValueLocation(gravityNode);
- secondary.setMessage("Incompatible direction here");
- location.setSecondary(secondary);
}
}
}
diff --git a/lint/libs/lint-model/src/main/java/com/android/tools/lint/model/PathVariables.kt b/lint/libs/lint-model/src/main/java/com/android/tools/lint/model/PathVariables.kt
index ad0716b637..e6b572744d 100644
--- a/lint/libs/lint-model/src/main/java/com/android/tools/lint/model/PathVariables.kt
+++ b/lint/libs/lint-model/src/main/java/com/android/tools/lint/model/PathVariables.kt
@@ -113,10 +113,17 @@ class PathVariables {
}
val file = File(path)
- return if (relativeTo != null && !file.isAbsolute) {
- File(relativeTo, path)
+ if (relativeTo != null && !file.isAbsolute) {
+ // Don't create paths like foo/bar/../something -- instead create foo/something
+ if (path.startsWith("../") || path.startsWith("..\\")) {
+ val parentFile = relativeTo.parentFile
+ if (parentFile != null) {
+ return File(parentFile, path.substring(3))
+ }
+ }
+ return File(relativeTo, path)
} else {
- file
+ return file
}
}
diff --git a/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/LintFixVerifier.kt b/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/LintFixVerifier.kt
index 2b5dd86302..7b426945a4 100644
--- a/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/LintFixVerifier.kt
+++ b/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/LintFixVerifier.kt
@@ -15,40 +15,27 @@
*/
package com.android.tools.lint.checks.infrastructure
-import com.android.SdkConstants.ANDROID_NS_NAME
-import com.android.SdkConstants.ANDROID_URI
-import com.android.SdkConstants.AUTO_URI
import com.android.SdkConstants.DOT_XML
-import com.android.SdkConstants.TOOLS_URI
-import com.android.SdkConstants.XMLNS_PREFIX
import com.android.ide.common.xml.XmlPrettyPrinter
import com.android.tools.lint.LintFixPerformer
-import com.android.tools.lint.LintFixPerformer.Companion.createAnnotationFix
import com.android.tools.lint.LintFixPerformer.Companion.getLocation
import com.android.tools.lint.checks.infrastructure.TestLintResult.Companion.getDiff
import com.android.tools.lint.detector.api.Incident
import com.android.tools.lint.detector.api.LintFix
-import com.android.tools.lint.detector.api.LintFix.AnnotateFix
import com.android.tools.lint.detector.api.LintFix.GroupType
import com.android.tools.lint.detector.api.LintFix.LintFixGroup
import com.android.tools.lint.detector.api.LintFix.SetAttribute
import com.android.tools.lint.detector.api.LintFix.ShowUrl
-import com.android.utils.PositionXmlParser
import com.android.utils.XmlUtils
+import com.google.common.base.Splitter
import com.google.common.collect.Lists
import org.junit.Assert.assertEquals
-import org.junit.Assert.assertNotNull
import org.junit.Assert.assertTrue
-import org.junit.Assert.fail
-import org.w3c.dom.Attr
-import org.w3c.dom.Document
-import org.w3c.dom.Element
-import org.w3c.dom.Node
import org.xml.sax.SAXException
import java.io.File
import java.io.IOException
+import java.util.Base64
import java.util.regex.Pattern
-import javax.xml.parsers.ParserConfigurationException
/**
* Verifier which can simulate IDE quickfixes and check fix data.
@@ -193,12 +180,9 @@ class LintFixVerifier(private val task: TestLintTask, state: TestResultState) {
listOf(fix)
}
for (lintFix in list) {
- val location = getLocation(incident)
+ val location = getLocation(incident, lintFix)
val project = incident.project
val targetPath = project?.getDisplayPath(location.file) ?: location.file.path
- val file = findTestFile(targetPath) ?: error("Didn't find test file $targetPath")
- var before = file.getContents() ?: continue
- assertNotNull(file.targetPath, before)
if (lintFix is LintFix.DataMap && diffs != null) {
// Doesn't edit file, but include in diffs so fixes can verify the
// correct data is passed
@@ -207,36 +191,47 @@ class LintFixVerifier(private val task: TestLintTask, state: TestResultState) {
appendShowUrl(incident, lintFix, diffs)
}
var reformat = reformat
- var after = applyFix(incident, lintFix, before) ?: continue
+
+ val initial: MutableMap<String, String> = HashMap()
+ val edited: MutableMap<String, String> = HashMap()
+
+ if (!applyFix(incident, lintFix, initial, edited)) {
+ continue
+ }
+
if (reformat == null && haveSetAttribute(lintFix)) {
reformat = true
}
if (expectedFile != null) {
+ val after = edited[targetPath]!!
assertEquals(
expectedFile.getContents()!!.trimIndent(),
after.trimIndent()
)
}
if (diffs != null) {
- if (reformat != null && reformat &&
- incident.getDisplayPath().endsWith(DOT_XML)
- ) {
- try {
- before = XmlPrettyPrinter.prettyPrint(
- XmlUtils.parseDocument(before, true), true
- )
- after = XmlPrettyPrinter.prettyPrint(
- XmlUtils.parseDocument(after, true), true
- )
- } catch (e: SAXException) {
- throw RuntimeException(e)
- } catch (e: IOException) {
- throw RuntimeException(e)
+ if (reformat != null && reformat) {
+ for ((f, contents) in edited) {
+ if (!f.endsWith(DOT_XML)) {
+ continue
+ }
+ try {
+ initial[f] = XmlPrettyPrinter.prettyPrint(
+ XmlUtils.parseDocument(initial[f]!!, true), true
+ )
+ edited[f] = XmlPrettyPrinter.prettyPrint(
+ XmlUtils.parseDocument(contents, true), true
+ )
+ } catch (e: SAXException) {
+ throw RuntimeException(e)
+ } catch (e: IOException) {
+ throw RuntimeException(e)
+ }
}
}
- appendDiff(incident, lintFix.displayName, before, after, diffs)
+ appendDiff(incident, lintFix.getDisplayName(), initial, edited, diffs)
}
- val name = lintFix.displayName
+ val name = lintFix.getDisplayName()
if (fixName != null && fixName != name) {
if (!names.contains(name)) {
names.add(name)
@@ -251,141 +246,93 @@ class LintFixVerifier(private val task: TestLintTask, state: TestResultState) {
private fun applyFix(
incident: Incident,
lintFix: LintFix,
- before: String
- ): String? {
- if (lintFix is LintFix.ReplaceString) {
- return checkReplaceString(lintFix, incident, before)
- } else if (lintFix is SetAttribute) {
- return checkSetAttribute(lintFix, before, incident)
- } else if (lintFix is AnnotateFix) {
- return checkAnnotationFix(lintFix, before, incident)
- } else if (lintFix is LintFixGroup && lintFix.type == GroupType.COMPOSITE) {
- var cumulative = before
- for (nested in lintFix.fixes) {
- val after = applyFix(incident, nested, cumulative) ?: return null
- cumulative = after
- }
- return cumulative
- }
- return null
- }
-
- private fun checkAnnotationFix(
- fix: AnnotateFix,
- contents: String,
- incident: Incident
- ): String? {
- val replaceFix = createAnnotationFix(
- fix, if (fix.range != null) fix.range else incident.location, contents
- )
- return checkReplaceString(replaceFix, incident, contents)
- }
-
- private fun checkSetAttribute(
- setFix: SetAttribute,
- contents: String,
- incident: Incident
- ): String {
- val location = if (setFix.range != null) setFix.range else incident.location
- location ?: return contents
- val start = location.start ?: return contents
- return try {
- val document: Document = PositionXmlParser.parse(contents)
- var node = PositionXmlParser.findNodeAtOffset(document, start.offset)
- assertNotNull("No node found at offset " + start.offset, node)
- node!!
- if (node.nodeType == Node.ATTRIBUTE_NODE) {
- node = (node as Attr).ownerElement
- } else if (node.nodeType != Node.ELEMENT_NODE) {
- // text, comments
- node = node.parentNode
- }
- if (node == null || node.nodeType != Node.ELEMENT_NODE) {
- fail(
- "Didn't find element at offset ${start.offset} (line ${start.line}1, " +
- "column ${start.column}1) in ${incident.getDisplayPath()}:\n$contents"
- )
- }
- val element = node as Element
- var value = setFix.value
- val namespace = setFix.namespace
- if (value == null) {
- if (namespace != null) {
- element.removeAttributeNS(namespace, setFix.attribute)
- } else {
- element.removeAttribute(setFix.attribute)
- }
- } else {
- // Indicate the caret position by "|"
- if (setFix.dot >= 0 && setFix.dot <= value.length) {
- value = if (setFix.mark >= 0 && setFix.mark != setFix.dot) {
- // Selection
- assert(setFix.mark < setFix.dot)
- value.substring(0, setFix.mark) + "[" + value.substring(setFix.mark, setFix.dot) +
- "]|" + value.substring(setFix.dot)
+ before: MutableMap<String, String>,
+ after: MutableMap<String, String>
+ ): Boolean {
+ if (LintFixPerformer.isEditingFix(lintFix) || lintFix is LintFixGroup) {
+ val edits = getLeafFixes(lintFix)
+ val performer = object : LintFixPerformer(
+ client,
+ printStatistics = false,
+ requireAutoFixable = false,
+ includeMarkers = task.includeSelectionMarkers
+ ) {
+ override fun writeFile(pendingFile: PendingEditFile, contents: ByteArray?) {
+ val project = incident.project
+ val targetPath = project?.getDisplayPath(pendingFile.file) ?: pendingFile.file.path
+ val initial = findTestFile(targetPath)?.getContents() ?: "" // creating new file
+ before[targetPath] = initial
+ if (contents == null) {
+ after[targetPath] = ""
} else {
- // Just caret
- value.substring(0, setFix.dot) + "|" + value.substring(setFix.dot)
+ val base64 = "base64: " + Base64.getEncoder().encodeToString(contents)
+ after[targetPath] = Splitter.fixedLength(60).split(base64).joinToString("\n") { " $it" }
}
}
- if (namespace != null) {
- // Workaround for the fact that the namespace-setter method
- // doesn't seem to work on these documents
- var prefix = document.lookupPrefix(namespace)
- if (prefix == null) {
- val base = when {
- ANDROID_URI == namespace -> ANDROID_NS_NAME
- TOOLS_URI == namespace -> "tools"
- AUTO_URI == namespace -> "app"
- else -> "ns"
- }
- val root = document.documentElement
- var index = 1
- while (true) {
- prefix = base + if (index == 1) "" else index.toString()
- if (!root.hasAttribute(XMLNS_PREFIX + prefix)) {
- break
- }
- index++
- }
- root.setAttribute(XMLNS_PREFIX + prefix, namespace)
- }
- element.setAttribute(prefix + ":" + setFix.attribute, value)
- } else {
- element.setAttribute(setFix.attribute, value)
+
+ override fun writeFile(pendingFile: PendingEditFile, contents: String) {
+ val project = incident.project
+ val targetPath = project?.getDisplayPath(pendingFile.file) ?: pendingFile.file.path
+ val initial = findTestFile(targetPath)?.getContents() ?: "" // creating new file
+ before[targetPath] = initial
+ after[targetPath] = contents
}
}
- XmlPrettyPrinter.prettyPrint(document, true)
- } catch (e: ParserConfigurationException) {
- throw RuntimeException(e)
- } catch (e: SAXException) {
- throw RuntimeException(e)
- } catch (e: IOException) {
- throw RuntimeException(e)
+ performer.fix(incident, edits)
+ return true
}
+ return false
}
private fun appendDiff(
incident: Incident,
fixDescription: String?,
- before: String,
- after: String,
+ initial: MutableMap<String, String>,
+ edited: MutableMap<String, String>,
diffs: StringBuilder
) {
- val diff = getDiff(before, after, diffWindow)
- if (diff.isNotEmpty()) {
- val targetPath = incident.getDisplayPath().replace(File.separatorChar, '/')
- diffs.append("Fix for ")
- .append(targetPath)
- .append(" line ")
- .append(incident.line + 1)
- .append(": ")
- if (fixDescription != null) {
- diffs.append(fixDescription).append(":\n")
- } else {
- diffs.append("\n")
+ var first = true
+
+ // List quickfixes in alphabetical order, except the file that the
+ // incident location is associated with goes first
+ val incidentPath = incident.getDisplayPath()
+ val comparator = object : Comparator<String> {
+ override fun compare(o1: String, o2: String): Int {
+ val v1 = if (o1 == incidentPath) 0 else 1
+ val v2 = if (o2 == incidentPath) 0 else 1
+ val delta = v1 - v2
+ if (delta != 0) {
+ return delta
+ }
+ return v1.compareTo(v2)
+ }
+ }
+ val sortedFiles = edited.keys.sortedWith(comparator)
+
+ for (file in sortedFiles) {
+ val after = edited[file]!!
+ val before = initial[file]!!
+ val diff = getDiff(before, after, diffWindow)
+ if (diff.isNotEmpty()) {
+ val targetPath = file.replace(File.separatorChar, '/')
+ if (first) {
+ diffs.append("Fix for ")
+ .append(incidentPath)
+ .append(" line ")
+ .append(incident.line + 1)
+ .append(":")
+ if (fixDescription != null) {
+ first = false
+ diffs.append(" ").append(fixDescription).append(":\n")
+ } else {
+ diffs.append("\n")
+ }
+ }
+ if (targetPath != incidentPath) {
+ diffs.append(targetPath).append(":\n")
+ }
+ diffs.append(diff).append("\n")
}
- diffs.append(diff).append("\n")
}
}
@@ -400,7 +347,7 @@ class LintFixVerifier(private val task: TestLintTask, state: TestResultState) {
.append(" line ")
.append(incident.line + 1)
.append(": ")
- val fixDescription = fix.displayName
+ val fixDescription = fix.getDisplayName()
if (fixDescription != null) {
diffs.append(fixDescription).append(":\n")
}
@@ -415,7 +362,7 @@ class LintFixVerifier(private val task: TestLintTask, state: TestResultState) {
.append(" line ")
.append(incident.line + 1)
.append(": ")
- val fixDescription = map.displayName
+ val fixDescription = map.getDisplayName()
if (fixDescription != null) {
diffs.append(fixDescription).append(":\n")
}
@@ -470,200 +417,19 @@ class LintFixVerifier(private val task: TestLintTask, state: TestResultState) {
return false
}
- private fun checkReplaceString(
- replaceFix: LintFix.ReplaceString,
- incident: Incident,
- contents: String
- ): String? {
- val oldPattern = replaceFix.oldPattern
- val oldString = replaceFix.oldString
- val location = if (replaceFix.range != null) replaceFix.range else incident.location
- location ?: return contents
- val start = location.start ?: return contents
- val end = location.end ?: return contents
- val locationRange = contents.substring(start.offset, end.offset)
- var startOffset: Int
- var endOffset: Int
- var replacement = replaceFix.replacement
- if (oldString == null && oldPattern == null) {
- // Replace the whole range
- startOffset = start.offset
- endOffset = end.offset
-
- // See if there's nothing left on the line; if so, delete the whole line
- var allSpace = true
- for (offset in replacement.indices) {
- val c = contents[offset]
- if (!Character.isWhitespace(c)) {
- allSpace = false
- break
- }
- }
- if (allSpace) {
- var lineBegin = startOffset
- while (lineBegin > 0) {
- val c = contents[lineBegin - 1]
- if (c == '\n') {
- break
- } else if (!Character.isWhitespace(c)) {
- allSpace = false
- break
- }
- lineBegin--
- }
- var lineEnd = endOffset
- while (lineEnd < contents.length) {
- val c = contents[lineEnd]
- lineEnd++
- if (c == '\n') {
- break
- }
- }
- if (allSpace) {
- startOffset = lineBegin
- endOffset = lineEnd
- }
- }
- } else if (oldString != null) {
- val index = locationRange.indexOf(oldString)
- when {
- index != -1 -> {
- startOffset = start.offset + index
- endOffset = start.offset + index + oldString.length
- }
- oldString == LintFix.ReplaceString.INSERT_BEGINNING -> {
- startOffset = start.offset
- endOffset = startOffset
- }
- oldString == LintFix.ReplaceString.INSERT_END -> {
- startOffset = end.offset
- endOffset = startOffset
- }
- else -> {
- fail(
- "Did not find \"" +
- oldString +
- "\" in \"" +
- locationRange +
- "\" as suggested in the quickfix. Consider calling " +
- "ReplaceStringBuilder#range() to set a larger range to " +
- "search than the default highlight range."
- )
- return null
- }
- }
- } else {
- oldPattern ?: return null
- val pattern = Pattern.compile(oldPattern)
- val matcher = pattern.matcher(locationRange)
- if (!matcher.find()) {
- fail(
- "Did not match pattern \"" +
- oldPattern +
- "\" in \"" +
- locationRange +
- "\" as suggested in the quickfix"
- )
- return null
- } else {
- startOffset = start.offset
- endOffset = startOffset
- if (matcher.groupCount() > 0) {
- if (oldPattern.contains("target")) {
- try {
- startOffset += matcher.start("target")
- endOffset += matcher.end("target")
- } catch (ignore: IllegalArgumentException) {
- // Occurrence of "target" not actually a named group
- startOffset += matcher.start(1)
- endOffset += matcher.end(1)
- }
- } else {
- startOffset += matcher.start(1)
- endOffset += matcher.end(1)
- }
- } else {
- startOffset += matcher.start()
- endOffset += matcher.end()
- }
- replacement = replaceFix.expandBackReferences(matcher)
- }
- }
- if (replaceFix.shortenNames) {
- // This isn't fully shortening names, it's only removing fully qualified names
- // for symbols already imported.
- //
- // Also, this will not correctly handle some conflicts. This is only used for
- // unit testing lint fixes, not for actually operating on code; for that we're using
- // IntelliJ's built in import cleanup when running in the IDE.
- val imported: MutableSet<String> = HashSet()
- for (line in contents.split("\n").toTypedArray()) {
- if (line.startsWith("package ")) {
- var to = line.indexOf(';')
- if (to == -1) {
- to = line.length
- }
- imported.add(line.substring("package ".length, to).trim { it <= ' ' } + ".")
- } else if (line.startsWith("import ")) {
- var from = "import ".length
- if (line.startsWith("static ")) {
- from += " static ".length
- }
- var to = line.indexOf(';')
- if (to == -1) {
- to = line.length
- }
- if (line[to - 1] == '*') {
- to--
- }
- imported.add(line.substring(from, to).trim { it <= ' ' })
- }
- }
- for (full in imported) {
- var clz = full
- if (replacement.contains(clz)) {
- if (!clz.endsWith(".")) {
- val index = clz.lastIndexOf('.')
- if (index == -1) {
- continue
- }
- clz = clz.substring(0, index + 1)
- }
- replacement = replacement.replace(clz, "")
- }
- }
- }
- var s = contents.substring(0, startOffset) + replacement + contents.substring(endOffset)
-
- // Insert selection/caret markers if configured for this fix
- val selectPattern = replaceFix.selectPattern
- if (selectPattern != null) {
- val pattern = Pattern.compile(selectPattern)
- val matcher = pattern.matcher(s)
- if (matcher.find(start.offset)) {
- val selectStart: Int
- val selectEnd: Int
- if (matcher.groupCount() > 0) {
- selectStart = matcher.start(1)
- selectEnd = matcher.end(1)
- } else {
- selectStart = matcher.start()
- selectEnd = matcher.end()
- }
- s = if (selectStart == selectEnd) {
- s.substring(0, selectStart) + "|" + s.substring(selectEnd)
- } else {
- (
- s.substring(0, selectStart) +
- "[" +
- s.substring(selectStart, selectEnd) +
- "]" +
- s.substring(selectEnd)
- )
+ private fun getLeafFixes(fix: LintFix): List<LintFix> {
+ val flattened: MutableList<LintFix> = mutableListOf()
+ fun flatten(fix: LintFix) {
+ if (LintFixPerformer.isEditingFix(fix)) {
+ flattened.add(fix)
+ } else if (fix is LintFixGroup && fix.type == GroupType.COMPOSITE) {
+ for (nested in fix.fixes) {
+ flatten(nested)
}
}
}
- return s
+ flatten(fix)
+ return flattened
}
}
}
diff --git a/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintClient.java b/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintClient.java
index e283cae843..e21784a802 100644
--- a/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintClient.java
+++ b/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintClient.java
@@ -28,6 +28,7 @@ import static com.android.ide.common.rendering.api.ResourceNamespace.RES_AUTO;
import static com.android.tools.lint.checks.infrastructure.KotlinClasspathKt.findKotlinStdlibPath;
import static com.android.tools.lint.checks.infrastructure.LintTestUtils.checkTransitiveComparator;
import static java.io.File.separatorChar;
+import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@@ -54,11 +55,14 @@ import com.android.support.AndroidxNameUtils;
import com.android.tools.lint.LintCliClient;
import com.android.tools.lint.LintCliFlags;
import com.android.tools.lint.LintCliXmlParser;
+import com.android.tools.lint.LintFixPerformer;
import com.android.tools.lint.LintResourceRepository;
import com.android.tools.lint.LintStats;
import com.android.tools.lint.Reporter;
import com.android.tools.lint.TextReporter;
import com.android.tools.lint.XmlFileType;
+import com.android.tools.lint.XmlReader;
+import com.android.tools.lint.XmlWriter;
import com.android.tools.lint.client.api.CircularDependencyException;
import com.android.tools.lint.client.api.Configuration;
import com.android.tools.lint.client.api.ConfigurationHierarchy;
@@ -111,6 +115,7 @@ import com.intellij.psi.PsiErrorElement;
import com.intellij.psi.util.PsiTreeUtil;
import java.io.ByteArrayInputStream;
import java.io.File;
+import java.io.FileWriter;
import java.io.IOException;
import java.io.InputStream;
import java.io.StringWriter;
@@ -128,6 +133,7 @@ import java.util.regex.Pattern;
import java.util.stream.Collectors;
import kotlin.Pair;
import kotlin.io.FilesKt;
+import org.jetbrains.annotations.NotNull;
import org.jetbrains.kotlin.config.LanguageVersionSettings;
import org.jetbrains.uast.UFile;
import org.w3c.dom.Attr;
@@ -983,13 +989,6 @@ public class TestLintClient extends LintCliClient {
Issue issue = incident.getIssue();
assertNotNull(location);
- if (fix != null && !task.allowExceptions) {
- Throwable throwable = LintFix.getThrowable(fix, LintDriver.KEY_THROWABLE);
- if (throwable != null && this.firstThrowable == null) {
- this.firstThrowable = throwable;
- }
- }
-
// Ensure that we're inside a read action if we might need access to PSI.
// This is one heuristic; we could add more assertions elsewhere as needed.
if (context instanceof JavaContext
@@ -1054,6 +1053,10 @@ public class TestLintClient extends LintCliClient {
}
}
+ incident = checkIncidentSerialization(incident);
+
+ checkFix(fix, incident);
+
super.report(context, incident, format);
// Make sure errors are unique! See documentation for #allowDuplicates.
@@ -1129,12 +1132,54 @@ public class TestLintClient extends LintCliClient {
}
}
}
+ }
+
+ private Incident checkIncidentSerialization(@NotNull Incident incident) {
+ // Check persistence: serialize and deserialize the issue metadata and continue using the
+ // deserialized version. It also catches cases where a detector modifies the incident
+ // after reporting it.
+ try {
+ File xmlFile = File.createTempFile("incident", ".xml");
+ XmlWriter xmlWriter =
+ new XmlWriter(this, XmlFileType.INCIDENTS, new FileWriter(xmlFile));
+ xmlWriter.writeIncidents(Collections.singletonList(incident), emptyList());
+
+ // Read it back
+ List<Issue> issueList = singletonList(incident.getIssue());
+ //noinspection MissingVendor
+ IssueRegistry registry =
+ new IssueRegistry() {
+ @NotNull
+ @Override
+ public List<Issue> getIssues() {
+ return issueList;
+ }
+ };
+ XmlReader xmlReader = new XmlReader(this, registry, incident.getProject(), xmlFile);
+ incident = xmlReader.getIncidents().get(0);
+ //noinspection ResultOfMethodCallIgnored
+ xmlFile.delete();
+ } catch (IOException e) {
+ e.printStackTrace();
+ }
+ return incident;
+ }
+
+ /** Validity checks for the quickfix associated with the given incident */
+ private void checkFix(LintFix fix, Incident incident) {
+ if (fix != null && !task.allowExceptions) {
+ Throwable throwable = LintFix.getThrowable(fix, LintDriver.KEY_THROWABLE);
+ if (throwable != null && this.firstThrowable == null) {
+ this.firstThrowable = throwable;
+ }
+ }
if (fix instanceof LintFix.ReplaceString) {
LintFix.ReplaceString replaceFix = (LintFix.ReplaceString) fix;
- String oldPattern = replaceFix.oldPattern;
- String oldString = replaceFix.oldString;
- Location rangeLocation = replaceFix.range != null ? replaceFix.range : location;
+ String oldPattern = replaceFix.getOldPattern();
+ Location range = replaceFix.getRange();
+ String oldString = replaceFix.getOldString();
+ Location rangeLocation = range != null ? range : incident.getLocation();
String contents = readFile(rangeLocation.getFile()).toString();
Position start = rangeLocation.getStart();
Position end = rangeLocation.getEnd();
@@ -1153,7 +1198,7 @@ public class TestLintClient extends LintCliClient {
+ "\" in \""
+ locationRange
+ "\" as suggested in the quickfix for issue "
- + issue
+ + incident.getIssue()
+ " (text in range was \""
+ locationRange
+ "\")");
@@ -1168,9 +1213,52 @@ public class TestLintClient extends LintCliClient {
+ "\" in \""
+ locationRange
+ "\" as suggested in the quickfix for issue "
- + issue);
+ + incident.getIssue());
}
}
+
+ File file = range != null ? range.getFile() : incident.getLocation().getFile();
+ if (!file.isFile()) {
+ fail(file + " is not a file. Use fix().createFile instead of fix().replace.");
+ }
+ } else if (fix instanceof LintFix.CreateFileFix) {
+ LintFix.CreateFileFix createFix = (LintFix.CreateFileFix) fix;
+ File file = createFix.getFile();
+ if (createFix.getDelete()) {
+ assertTrue(file + " cannot be deleted; does not exist", file.exists());
+ } else if (file.exists()) {
+ fail("Cannot create file " + file + ": already exists");
+ }
+ }
+
+ // Make sure any source files referenced by the quick fixes are loaded
+ // for subsequent quickfix verifications (since those run after the
+ // test projects have been deleted)
+ if (fix != null) {
+ readFixFiles(incident, fix);
+ }
+ }
+
+ private void readFixFiles(Incident incident, LintFix fix) {
+ if (fix instanceof LintFix.LintFixGroup) {
+ for (LintFix f : ((LintFix.LintFixGroup) fix).getFixes()) {
+ readFixFiles(incident, f);
+ }
+ } else {
+ Location location = LintFixPerformer.Companion.getLocation(incident, fix);
+ File file = location.getFile();
+ if (file.isFile()) {
+ String displayName = fix.getDisplayName();
+ assertTrue(
+ "Quickfix file paths must be absolute: "
+ + file
+ + " from "
+ + (displayName != null
+ ? displayName
+ : fix.getClass().getSimpleName()),
+ file.isAbsolute());
+ getSourceText(file);
+ }
}
}
@@ -1286,7 +1374,7 @@ public class TestLintClient extends LintCliClient {
sb.append(String.format(format, args));
}
if (exception != null) {
- sb.append(exception.toString());
+ sb.append(exception);
}
System.err.println(sb);
diff --git a/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintResult.kt b/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintResult.kt
index d89756cd04..078d5846bd 100644
--- a/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintResult.kt
+++ b/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintResult.kt
@@ -937,8 +937,8 @@ class TestLintResult internal constructor(
*/
fun getDiff(before: String, after: String, windowSize: Int): String {
return getDiff(
- before.split("\n").toTypedArray(),
- after.split("\n").toTypedArray(),
+ if (before.isEmpty()) emptyArray() else before.split("\n").toTypedArray(),
+ if (after.isEmpty()) emptyArray() else after.split("\n").toTypedArray(),
windowSize
)
}
diff --git a/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintTask.java b/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintTask.java
index f882bac1f2..2042f222c5 100644
--- a/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintTask.java
+++ b/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintTask.java
@@ -138,6 +138,7 @@ public class TestLintTask {
boolean useTestConfiguration = true;
@Nullable ProjectDescription reportFrom = null;
boolean stripRoot = true;
+ boolean includeSelectionMarkers = true;
/** Creates a new lint test task */
public TestLintTask() {
@@ -609,6 +610,18 @@ public class TestLintTask {
}
/**
+ * Whether lint should insert selection markers in quickfix tests, using square brackets to show
+ * the selection range and `|` to show the caret position.
+ *
+ * @param includeSelectionMarkers true (the default) to show markers
+ * @return this, for constructor chaining
+ */
+ public TestLintTask includeSelectionMarkers(boolean includeSelectionMarkers) {
+ this.includeSelectionMarkers = includeSelectionMarkers;
+ return this;
+ }
+
+ /**
* Registers a hook to initialize the lint driver during test execution
*
* @param configurator the callback to configure the lint driver
diff --git a/lint/libs/lint-tests/src/test/java/com/android/tools/lint/LintFixPerformerTest.kt b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/LintFixPerformerTest.kt
index e8040056b1..af89b8c5d7 100644
--- a/lint/libs/lint-tests/src/test/java/com/android/tools/lint/LintFixPerformerTest.kt
+++ b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/LintFixPerformerTest.kt
@@ -43,7 +43,7 @@ class LintFixPerformerTest : TestCase() {
var output = ""
val printStatistics = expectedOutput != null
val performer = object : LintFixPerformer(client, printStatistics) {
- override fun writeFile(file: PendingEditFile, contents: String) {
+ override fun writeFile(pendingFile: PendingEditFile, contents: String) {
after = contents
}
diff --git a/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/ButtonDetectorTest.java b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/ButtonDetectorTest.java
index 3cc40ce829..1ceec1c15c 100644
--- a/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/ButtonDetectorTest.java
+++ b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/ButtonDetectorTest.java
@@ -482,13 +482,17 @@ public class ButtonDetectorTest extends AbstractCheckTest {
""
+ "Fix for res/layout/buttonbar.xml line 9: Make borderless:\n"
+ "@@ -7 +7\n"
- + "+ style=\"?android:attr/buttonBarButtonStyle\"\n"
+ + "+ style=\"?android:attr/buttonBarStyle\"\n"
+ "@@ -11 +12\n"
+ "+ style=\"?android:attr/buttonBarButtonStyle\"\n"
+ + "@@ -16 +18\n"
+ + "+ style=\"?android:attr/buttonBarButtonStyle\"\n"
+ "Fix for res/layout/buttonbar.xml line 14: Make borderless:\n"
+ "@@ -7 +7\n"
- + "+ style=\"?android:attr/buttonBarButtonStyle\"\n"
+ + "+ style=\"?android:attr/buttonBarStyle\"\n"
+ "@@ -11 +12\n"
+ + "+ style=\"?android:attr/buttonBarButtonStyle\"\n"
+ + "@@ -16 +18\n"
+ "+ style=\"?android:attr/buttonBarButtonStyle\"");
}
diff --git a/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/FontDetectorTest.java b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/FontDetectorTest.java
index 5aa1a8358a..eb05fd7126 100644
--- a/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/FontDetectorTest.java
+++ b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/FontDetectorTest.java
@@ -222,7 +222,7 @@ public class FontDetectorTest extends AbstractCheckTest {
+ "1 errors, 0 warnings";
String expectedFix =
""
- + "Fix for res/font/font1.xml line 1: Set fontProviderAuthority=\"com.google.android.gms.fonts\":\n"
+ + "Fix for res/font/font1.xml line 2: Set missing attributes:\n"
+ "@@ -3 +3\n"
+ "+ android:fontProviderAuthority=\"com.google.android.gms.fonts\"\n"
+ "+ android:fontProviderCerts=\"@array/com_google_android_gms_fonts_certs\"\n"
@@ -254,11 +254,11 @@ public class FontDetectorTest extends AbstractCheckTest {
+ "1 errors, 0 warnings";
String expectedFix =
""
- + "Fix for res/font/font1.xml line 1: Set fontProviderAuthority:\n"
+ + "Fix for res/font/font1.xml line 2: Set missing attributes:\n"
+ "@@ -3 +3\n"
- + "+ android:fontProviderAuthority=\"[TODO]|\"\n"
+ + "+ android:fontProviderAuthority=\"TODO\"\n"
+ "+ android:fontProviderCerts=\"[TODO]|\"\n"
- + "+ android:fontProviderPackage=\"[TODO]|\"\n";
+ + "+ android:fontProviderPackage=\"TODO\"";
lint().files(
manifest()
.minSdk(
@@ -287,7 +287,7 @@ public class FontDetectorTest extends AbstractCheckTest {
+ "1 errors, 0 warnings";
String expectedFix =
""
- + "Fix for res/font/font1.xml line 1: Set fontProviderAuthority=\"com.google.android.gms.fonts\":\n"
+ + "Fix for res/font/font1.xml line 2: Set missing attributes:\n"
+ "@@ -3 +3\n"
+ "+ app:fontProviderAuthority=\"com.google.android.gms.fonts\"\n"
+ "+ app:fontProviderCerts=\"@array/com_google_android_gms_fonts_certs\"\n"
@@ -317,7 +317,7 @@ public class FontDetectorTest extends AbstractCheckTest {
+ "1 errors, 0 warnings";
String expectedFix =
""
- + "Fix for res/font/font1.xml line 2: Replace with com.google.android.gms.fonts:\n"
+ + "Fix for res/font/font1.xml line 3: Replace with com.google.android.gms.fonts:\n"
+ "@@ -3 +3\n"
+ "- app:fontProviderAuthority=\"com.google.android.gms.helpme\"\n"
+ "+ app:fontProviderAuthority=\"com.google.android.gms.fonts\"\n";
diff --git a/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/IgnoreWithoutReasonDetectorTest.kt b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/IgnoreWithoutReasonDetectorTest.kt
index 1e870c4c5f..8c78d86c73 100644
--- a/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/IgnoreWithoutReasonDetectorTest.kt
+++ b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/IgnoreWithoutReasonDetectorTest.kt
@@ -228,7 +228,7 @@ class IgnoreWithoutReasonDetectorTest {
Fix for src/foo/MyTest.java line 8: Give reason:
@@ -8 +8
- @Test @Ignore void something() {
- + @Test @Ignore("[TODO]") void something() {
+ + @Test @Ignore("[TODO]|") void something() {
"""
)
}
diff --git a/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/infrastructure/LintFixVerifierTest.kt b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/infrastructure/LintFixVerifierTest.kt
index 1bc0c349c7..110df45d2b 100644
--- a/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/infrastructure/LintFixVerifierTest.kt
+++ b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/infrastructure/LintFixVerifierTest.kt
@@ -21,6 +21,7 @@ import com.android.testutils.TestUtils
import com.android.tools.lint.checks.infrastructure.TestFiles.gradle
import com.android.tools.lint.checks.infrastructure.TestFiles.java
import com.android.tools.lint.checks.infrastructure.TestFiles.kotlin
+import com.android.tools.lint.checks.infrastructure.TestFiles.source
import com.android.tools.lint.checks.infrastructure.TestLintTask.lint
import com.android.tools.lint.client.api.IssueRegistry
import com.android.tools.lint.client.api.UElementHandler
@@ -33,10 +34,12 @@ import com.android.tools.lint.detector.api.LintFix
import com.android.tools.lint.detector.api.Location
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.UImportStatement
import org.jetbrains.uast.UMethod
+import org.jetbrains.uast.getContainingUClass
import org.jetbrains.uast.getParentOfType
import org.junit.Test
import java.io.File
@@ -155,7 +158,8 @@ class LintFixVerifierTest {
// Dummy Gradle File
apply plugin: 'java'
"""
- ).indented()
+ ).indented(),
+ source("delete_me.txt", "Delete\nThis\nFile")
)
.sdkHome(TestUtils.getSdk().toFile())
.issues(*LintFixVerifierRegistry().issues.toTypedArray())
@@ -177,15 +181,143 @@ class LintFixVerifierTest {
@@ -3 +3
- public void oldName() {
+ public void renamedMethod() {
- Fix for src/main/java/test/pkg/Test.java line 5: Update build.gradle:
+ Fix for src/main/java/test/pkg/Test.java line 5: Update files:
+ @@ -3 +3
+ - public void oldName() {
+ + public void renamedMethod() {
+ data.bin:
+ @@ -1 +1
+ + base64: AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+ + AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+ + AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+ + AAAAAAAAAAAAAAAAAAAAAAAAAAAA
+ delete_me.txt:
+ @@ -1 +1
+ - Delete
+ - This
+ - File
+ build.gradle:
@@ -2 +2
- apply plugin: 'java'
@@ -3 +2
+ apply plugin: 'kotlin'
+ new.txt:
+ @@ -1 +1
+ + First line in [new]| file.
+ + Second line.
+ + The End.
"""
)
}
+ @Test
+ fun testMultipleEditOperationsOrder() {
+ // This test makes sure that multiple quickfixes applied to the same file are applied correctly
+ lint()
+ .files(
+ TestFiles.manifest().minSdk(14),
+ java(
+ """
+ package androidx;
+
+ import android.content.res.ColorStateList;
+ import android.os.Build;
+ import android.view.View;
+
+ /**
+ * Test class containing unsafe method references.
+ */
+ @SuppressWarnings("unused")
+ public class AutofixUnsafeVoidMethodReferenceJava {
+
+ /**
+ * Unsafe reference to a new API with an SDK_INT check that satisfies the NewApi lint.
+ */
+ void unsafeReferenceWithSdkCheck(View view) {
+ if (Build.VERSION.SDK_INT > 23) {
+ ColorStateList tint = new ColorStateList(null, null);
+ view.setBackgroundTintList(tint);
+ }
+ }
+ }
+ """
+ ).indented(),
+ )
+ .sdkHome(TestUtils.getSdk().toFile())
+ .issues(ClassVerificationFailureDetector.ISSUE)
+ .testModes(TestMode.DEFAULT)
+ .run()
+ .expect(
+ """
+ src/androidx/AutofixUnsafeVoidMethodReferenceJava.java:19: Error: This call references a method added in API level 21; however, the containing class androidx.AutofixUnsafeVoidMethodReferenceJava is reachable from earlier API levels and will fail run-time class verification. [_ClassVerificationFailure]
+ view.setBackgroundTintList(tint);
+ ~~~~~~~~~~~~~~~~~~~~~
+ 1 errors, 0 warnings
+ """
+ ).expectFixDiffs(
+ """
+ Fix for src/androidx/AutofixUnsafeVoidMethodReferenceJava.java line 19: Extract to static inner class:
+ @@ -19 +19
+ - view.setBackgroundTintList(tint);
+ + Api21Impl.setBackgroundTintList(view, tint);
+ @@ -22 +22
+ + @RequiresApi(21)
+ + static class Api21Impl {
+ + private Api21Impl() {
+ + // This class is non-instantiable.
+ + }
+ +
+ + static void setBackgroundTintList(View obj, ColorStateList tint) {
+ + obj.setBackgroundTintList(tint);
+ + }
+ """
+ )
+ }
+
+ class ClassVerificationFailureDetector : Detector(), SourceCodeScanner {
+ override fun getApplicableMethodNames(): List<String> = listOf("setBackgroundTintList")
+ @Suppress("BooleanLiteralArgument")
+ override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
+ val part1 = fix()
+ .replace()
+ .range(context.getLocation(node))
+ .with("Api21Impl.setBackgroundTintList(view, tint)")
+ .build()
+ val part2 = fix()
+ .replace()
+ .range(context.getLocation(node.getContainingUClass()!!.lastChild))
+ .beginning()
+ .with(
+ " @RequiresApi(21)\n" +
+ " static class Api21Impl {\n" +
+ " private Api21Impl() {\n" +
+ " // This class is non-instantiable.\n" +
+ " }\n" +
+ "\n" +
+ " static void setBackgroundTintList(View obj, ColorStateList tint) {\n" +
+ " obj.setBackgroundTintList(tint);\n" +
+ " }\n"
+ )
+ .build()
+ val fix = fix().name("Extract to static inner class").composite(part1, part2)
+ context.report(
+ ISSUE, context.getCallLocation(node, false, false),
+ "This call references a method added in API level 21; however, the containing class " +
+ "androidx.AutofixUnsafeVoidMethodReferenceJava is reachable from earlier API levels and " +
+ "will fail run-time class verification.",
+ fix
+ )
+ }
+
+ companion object {
+ val ISSUE = Issue.create(
+ "_ClassVerificationFailure", "Blah blah", "Blah blah blah",
+ Category.CORRECTNESS, 5, Severity.ERROR,
+ Implementation(ClassVerificationFailureDetector::class.java, Scope.JAVA_FILE_SCOPE)
+ ).setAndroidSpecific(true)
+ }
+ }
+
// Copied from above bug report:
// https://issuetracker.google.com/80491636
// which in turn looks like it comes from
@@ -255,18 +387,18 @@ class LintFixVerifierDetector : Detector(), Detector.UastScanner {
node: UCallExpression,
method: PsiMethod
) {
+ val parentMethod = node.getParentOfType<UMethod>(
+ UMethod::class.java,
+ false
+ )
+ val fix = fix()
+ .name("Rename Containing Method")
+ .replace()
+ .all()
+ .with("renamedMethod")
+ .range(context.getNameLocation(parentMethod!!))
+ .build()
if (method.name == "renameMethodNameInstead") {
- val parentMethod = node.getParentOfType<UMethod>(
- UMethod::class.java,
- false
- )
- val fix = fix()
- .name("Rename Containing Method")
- .replace()
- .all()
- .with("renamedMethod")
- .range(context.getNameLocation(parentMethod!!))
- .build()
context.report(
ISSUE, context.getLocation(node),
"This error has a quickfix which edits parent method name instead", fix
@@ -286,18 +418,49 @@ class LintFixVerifierDetector : Detector(), Detector.UastScanner {
val start = source.indexOf("java")
val end = start + 4
val range = Location.Companion.create(file, source, start, end)
- val fix = fix()
+
+ // Test modifying a file other than the one containing the incident
+ val gradleFix = fix()
.name("Update build.gradle")
.replace()
.all()
.with("kotlin")
.range(range)
.build()
+
+ // Test creating a new file too
+ val newFileFix = fix()
+ .name("Create file")
+ .newFile(
+ File(range.file.parentFile, "new.txt"),
+ "First line in new file.\nSecond line.\nThe End."
+ )
+ .select("(new)")
+ .build()
+
+ // And a new binary
+ val newBinaryFix = fix()
+ .name("Create blob")
+ .newFile(
+ File(range.file.parentFile, "data.bin"),
+ ByteArray(150) { 0 }
+ )
+ .build()
+
+ // And delete a file
+ val deleteFix = fix().deleteFile(File(file.parentFile, "delete_me.txt")).build()
+
+ // Test both updating a file separate from the incident location, but also
+ // updating multiple files in a single fix
+ val composite = fix().name("Update files").composite(
+ fix, gradleFix, newFileFix, newBinaryFix, deleteFix
+ )
+
context.report(
ISSUE,
context.getLocation(node),
"This error has a quickfix which edits something in a separate build.gradle file instead",
- fix
+ composite
)
}
}
diff --git a/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintResultTest.kt b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/infrastructure/TestLintResultTest.kt
index 2d519b310f..2d519b310f 100644
--- a/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintResultTest.kt
+++ b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/infrastructure/TestLintResultTest.kt