diff options
9 files changed, 99 insertions, 23 deletions
diff --git a/common/src/com/android/util/PositionXmlParser.java b/common/src/com/android/util/PositionXmlParser.java index 22ea4c1..54146db 100644 --- a/common/src/com/android/util/PositionXmlParser.java +++ b/common/src/com/android/util/PositionXmlParser.java @@ -382,12 +382,11 @@ public class PositionXmlParser { if (t == '\n') { newLine++; newColumn = 0; + } else if (!Character.isWhitespace(t)) { + break; } else { newColumn++; } - if (!Character.isWhitespace(t)) { - break; - } } if (textIndex == textLength) { textIndex = 0; // Whitespace node @@ -397,7 +396,7 @@ public class PositionXmlParser { } Position attributePosition = createPosition(line, column, - offset); + offset + textIndex); // Also set end range for retrieval in getLocation attributePosition.setEnd(createPosition(line, column, offset + textLength)); @@ -503,7 +502,7 @@ public class PositionXmlParser { // Compute new column position int column = 0; - for (int i = offset; i >= 0; i--, column++) { + for (int i = offset - 1; i >= 0; i--, column++) { if (mXml.charAt(i) == '\n') { break; } diff --git a/common/tests/src/com/android/util/PositionXmlParserTest.java b/common/tests/src/com/android/util/PositionXmlParserTest.java index 1573c90..ba560a6 100644 --- a/common/tests/src/com/android/util/PositionXmlParserTest.java +++ b/common/tests/src/com/android/util/PositionXmlParserTest.java @@ -22,6 +22,7 @@ import org.w3c.dom.Attr; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.NodeList; +import org.w3c.dom.Text; import java.io.BufferedOutputStream; import java.io.BufferedWriter; @@ -80,6 +81,7 @@ public class PositionXmlParserTest extends TestCase { Position start = parser.getPosition(attr); Position end = start.getEnd(); assertEquals(2, start.getLine()); + assertEquals(4, start.getColumn()); assertEquals(xml.indexOf("android:layout_width"), start.getOffset()); assertEquals(2, end.getLine()); String target = "android:layout_width=\"match_parent\""; @@ -91,6 +93,7 @@ public class PositionXmlParserTest extends TestCase { end = start.getEnd(); assertNull(end.getEnd()); assertEquals(6, start.getLine()); + assertEquals(4, start.getColumn()); assertEquals(xml.indexOf("<Button"), start.getOffset()); assertEquals(xml.indexOf("/>") + 2, end.getOffset()); assertEquals(10, end.getLine()); @@ -100,6 +103,7 @@ public class PositionXmlParserTest extends TestCase { start = parser.getPosition(button2); end = start.getEnd(); assertEquals(12, start.getLine()); + assertEquals(4, start.getColumn()); assertEquals(xml.indexOf("<Button", button1End), start.getOffset()); assertEquals(xml.indexOf("/>", start.getOffset()) + 2, end.getOffset()); assertEquals(16, end.getLine()); @@ -107,6 +111,52 @@ public class PositionXmlParserTest extends TestCase { file.delete(); } + public void testText() throws Exception { + String xml = + "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n" + + "<LinearLayout xmlns:android=\"http://schemas.android.com/apk/res/android\"\n" + + " android:layout_width=\"match_parent\"\n" + + " android:layout_height=\"wrap_content\"\n" + + " android:orientation=\"vertical\" >\n" + + "\n" + + " <Button\n" + + " android:id=\"@+id/button1\"\n" + + " android:layout_width=\"wrap_content\"\n" + + " android:layout_height=\"wrap_content\"\n" + + " android:text=\"Button\" />\n" + + " some text\n" + + "\n" + + "</LinearLayout>\n"; + PositionXmlParser parser = new PositionXmlParser(); + File file = File.createTempFile("parsertest", ".xml"); + file.deleteOnExit(); + Writer fw = new BufferedWriter(new FileWriter(file)); + fw.write(xml); + fw.close(); + Document document = parser.parse(new FileInputStream(file)); + assertNotNull(document); + + // Basic parsing heart beat tests + Element linearLayout = (Element) document.getElementsByTagName("LinearLayout").item(0); + assertNotNull(linearLayout); + NodeList buttons = document.getElementsByTagName("Button"); + assertEquals(1, buttons.getLength()); + final String ANDROID_URI = "http://schemas.android.com/apk/res/android"; + assertEquals("wrap_content", + linearLayout.getAttributeNS(ANDROID_URI, "layout_height")); + + // Check text positions + Element button = (Element) buttons.item(0); + Text text = (Text) button.getNextSibling(); + assertNotNull(text); + + // Check attribute positions + Position start = parser.getPosition(text); + assertEquals(11, start.getLine()); + assertEquals(10, start.getColumn()); + assertEquals(xml.indexOf("some text"), start.getOffset()); + } + public void testLineEndings() throws Exception { // Test for http://code.google.com/p/android/issues/detail?id=22925 String xml = diff --git a/lint/cli/src/com/android/tools/lint/Main.java b/lint/cli/src/com/android/tools/lint/Main.java index d3e4c8d..4d734a4 100644 --- a/lint/cli/src/com/android/tools/lint/Main.java +++ b/lint/cli/src/com/android/tools/lint/Main.java @@ -777,7 +777,7 @@ public class Main extends LintClient { warning.errorLine = warning.errorLine.replace('\t', ' '); int column = startPosition.getColumn(); if (column < 0) { - column = 1; + column = 0; for (int i = 0; i < warning.errorLine.length(); i++, column++) { if (!Character.isWhitespace(warning.errorLine.charAt(i))) { break; @@ -787,7 +787,7 @@ public class Main extends LintClient { StringBuilder sb = new StringBuilder(); sb.append(warning.errorLine); sb.append('\n'); - for (int i = 0; i < column - 1; i++) { + for (int i = 0; i < column; i++) { sb.append(' '); } sb.append('^'); diff --git a/lint/cli/src/com/android/tools/lint/TextReporter.java b/lint/cli/src/com/android/tools/lint/TextReporter.java index 4b8f7c3..8157f7c 100644 --- a/lint/cli/src/com/android/tools/lint/TextReporter.java +++ b/lint/cli/src/com/android/tools/lint/TextReporter.java @@ -67,9 +67,8 @@ class TextReporter extends Reporter { output.append('\n'); - if (warning.errorLine != null) { - output.append(warning.errorLine.trim()); - output.append('\n'); + if (warning.errorLine != null && warning.errorLine.length() > 0) { + output.append(warning.errorLine); } if (warning.location != null && warning.location.getSecondary() != null) { diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java index 3b2b5fd..306d5fe 100644 --- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java +++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java @@ -175,6 +175,7 @@ public class ClassContext extends Context { public Location getLocationForLine(int line, String patternStart, String patternEnd) { File sourceFile = getSourceFile(); if (sourceFile != null) { + // ASM line numbers are 1-based, and lint line numbers are 0-based return Location.create(sourceFile, getSourceContents(), line - 1, patternStart, patternEnd); } diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Location.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Location.java index 506728c..d18ab71 100644 --- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Location.java +++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Location.java @@ -273,10 +273,31 @@ public class Location { if (line == currentLine) { if (patternStart != null) { int index = contents.indexOf(patternStart, offset); + if (index == -1) { + // Allow some flexibility: peek at previous couple of lines + // as well (for example, bytecode line numbers are sometimes + // a few lines off from their location in a source file + // since they are attached to executable lines of code) + int lineStart = offset; + for (int i = 0; i < 4; i++) { + int prevLineStart = contents.lastIndexOf('\n', lineStart - 1); + if (prevLineStart == -1) { + break; + } + index = contents.indexOf(patternStart, prevLineStart); + if (index != -1 || prevLineStart == 0) { + break; + } + lineStart = prevLineStart; + } + } + if (index != -1) { int lineStart = contents.lastIndexOf('\n', index); if (lineStart == -1) { lineStart = 0; + } else { + lineStart++; // was pointing to the previous line's CR, not line start } int column = index - lineStart; if (patternEnd != null) { diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ApiDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ApiDetector.java index 1d65655..51257bb 100644 --- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ApiDetector.java +++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ApiDetector.java @@ -168,7 +168,8 @@ public class ApiDetector extends LayoutDetector implements Detector.ClassScanner String message = String.format( "Class requires API level %1$d (current min is %2$d): %3$s", api, minSdk, fqcn); - report(context, message, var.start); + report(context, message, var.start, + className.substring(className.lastIndexOf('/') + 1), null); } } @@ -191,7 +192,7 @@ public class ApiDetector extends LayoutDetector implements Detector.ClassScanner "Class requires API level %1$d (current min is %2$d): %3$s", api, minSdk, fqcn); AbstractInsnNode first = nodes.size() > 0 ? nodes.get(0) : null; - report(context, message, first); + report(context, message, first, null, null); } } } @@ -221,7 +222,7 @@ public class ApiDetector extends LayoutDetector implements Detector.ClassScanner String message = String.format( "Call requires API level %1$d (current min is %2$d): %3$s", api, minSdk, fqcn); - report(context, message, node); + report(context, message, node, name, null); } } else if (type == AbstractInsnNode.FIELD_INSN) { FieldInsnNode node = (FieldInsnNode) instruction; @@ -233,7 +234,7 @@ public class ApiDetector extends LayoutDetector implements Detector.ClassScanner String message = String.format( "Field requires API level %1$d (current min is %2$d): %3$s", api, minSdk, fqcn); - report(context, message, node); + report(context, message, node, name, null); } } else if (type == AbstractInsnNode.LDC_INSN) { LdcInsnNode node = (LdcInsnNode) instruction; @@ -247,7 +248,8 @@ public class ApiDetector extends LayoutDetector implements Detector.ClassScanner String message = String.format( "Class requires API level %1$d (current min is %2$d): %3$s", api, minSdk, fqcn); - report(context, message, node); + report(context, message, node, + className.substring(className.lastIndexOf('/') + 1), null); } } } @@ -278,9 +280,10 @@ public class ApiDetector extends LayoutDetector implements Detector.ClassScanner return -1; } - private void report(final ClassContext context, String message, AbstractInsnNode node) { + private void report(final ClassContext context, String message, AbstractInsnNode node, + String patternStart, String patternEnd) { int lineNumber = node != null ? findLineNumber(node) : -1; - Location location = context.getLocationForLine(lineNumber, null, null); + Location location = context.getLocationForLine(lineNumber, patternStart, patternEnd); context.report(MISSING, location, message, null); } } diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/FieldGetterDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/FieldGetterDetector.java index ad48355..b94cbc7 100644 --- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/FieldGetterDetector.java +++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/FieldGetterDetector.java @@ -127,7 +127,9 @@ public class FieldGetterDetector extends Detector implements Detector.ClassScann Integer line = pair.getSecond(); Location location = null; if (source != null) { - location = Location.create(source, contents, line); + // ASM line numbers are 1-based, Lint needs 0-based + location = Location.create(source, contents, line - 1, name, + null); } else { location = Location.create(mContext.file); } @@ -164,6 +166,7 @@ public class FieldGetterDetector extends Detector implements Detector.ClassScann if (mPendingCalls == null) { mPendingCalls = new ArrayList<Pair<String,Integer>>(); } + // Line numbers should be 0-based mPendingCalls.add(Pair.of(name, mCurrentLine)); } } diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/FieldGetterDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/FieldGetterDetectorTest.java index e50e6a6..c00c1bb 100644 --- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/FieldGetterDetectorTest.java +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/FieldGetterDetectorTest.java @@ -27,11 +27,11 @@ public class FieldGetterDetectorTest extends AbstractCheckTest { public void test() throws Exception { assertEquals( - "GetterTest.java:48: Warning: Calling getter method getFoo1() on self is slower than field access\n" + - "GetterTest.java:49: Warning: Calling getter method getFoo2() on self is slower than field access\n" + - "GetterTest.java:53: Warning: Calling getter method isBar1() on self is slower than field access\n" + - "GetterTest.java:55: Warning: Calling getter method getFoo1() on self is slower than field access\n" + - "GetterTest.java:56: Warning: Calling getter method getFoo2() on self is slower than field access", + "GetterTest.java:47: Warning: Calling getter method getFoo1() on self is slower than field access\n" + + "GetterTest.java:48: Warning: Calling getter method getFoo2() on self is slower than field access\n" + + "GetterTest.java:52: Warning: Calling getter method isBar1() on self is slower than field access\n" + + "GetterTest.java:54: Warning: Calling getter method getFoo1() on self is slower than field access\n" + + "GetterTest.java:55: Warning: Calling getter method getFoo2() on self is slower than field access", lintProject( "bytecode/.classpath=>.classpath", |