issues-import's People
issues-import's Issues
Error messages show only the first line of a multi-line fix
Original issue created by [email protected] on 2012-06-11 at 10:28 PM
Error messages show only the first line of a multi-line fix. We need to think about how to present larger fixes, such as those for covariant equals where we make changes to the whole method.
Code review request
Original issue created by [email protected] on 2012-03-02 at 11:01 PM
Fixed a bug in which compiling files that contain multiple top-level classes causes NullPointerExceptions. Please take a look at revision 66f9b5bb0201 and let me know what you think.
Validate literal regular expressions
Original issue created by [email protected] on 2012-10-19 at 02:10 AM
When a string literal is passed for a parameter that is known to expect a regex (Pattern.compile, String.split, etc.), try to actually compile that regex, and fail the build if that throws an exception.
(Perhaps also when the value of some constant is passed, and that constant is set to a literal. Etc.)
Code review request
Original issue created by [email protected] on 2012-10-20 at 12:53 AM
Please take a look at commit 6f8ddb631890. I changed the ObjectEqualsSelfComparsion checker to also check for foo.equals(foo).
Code review request
Original issue created by [email protected] on 2011-09-30 at 06:24 PM
Several commits to fix problems, and to move the JSR-269 dependency out of the ASTvisitor
Code review request
Original issue created by [email protected] on 2012-09-11 at 11:21 AM
Branch name: longliteral
Purpose of code changes on this branch: Add checker for long literals ending with lower case ell, e.g. 123432l rather than 123432L. See #23
After the review, I'll merge this branch into: /master
Code review request
Original issue created by [email protected] on 2012-07-26 at 09:54 PM
Please take a look at my 3 commits from today (7/26). I reworked ErrorReportingJavaCompiler to scan compilation units all at once, so that file-level nodes like imports and package declarations are scanned.
Filler for Non-Existent Issue
Filler for non-existent Google Code issue 22.
This issue only exists to ensure that GitHub issues have the same IDs they had on Google Code. Please ignore it.
Code review request
Original issue created by [email protected] on 2012-09-04 at 10:55 AM
Branch name: suppress
Purpose of code changes on this branch:
Checks for incorrectly spelled @SuppressWarnings() annotation values ("deprecated" instead of "deprecation" - cleaned it up recently with several dozen instances in google3).
I've refactored some of the code from the FallThroughSuppression check
into an abstract superclass.
After the review, I'll merge this branch into:
/master
Check that equals() and hashCode() read the same fields
Original issue created by [email protected] on 2012-08-06 at 08:48 PM
If I add a new field to a class, I can remember to update equals() but forget to update hashCode(). How would we detect this? In many cases, both methods should read all fields of a class, but that's probably too strong a check. (For example, a List.equals() implementation might read no fields directly, preferring to operation on the public size() and get() methods.) A weaker but probably still useful check is that the two methods read the same set of fields. False positives are still possible. For example, the check would flag a field containing a cached hash code, which would likely be read in hashCode() but not equals(). This particular example is avoidable by requiring the hashCode() looks at a subset of the fields that equals() looks at. I conjecture that it's more common for programmers to update equals() but forget to update hashCode() than vice versa, so this further weakened check would likely still catch most problems.
Code review request
Original issue created by [email protected] on 2011-09-27 at 10:30 PM
Branch name:
fixmultipleimports
Purpose of code changes on this branch:
Fixed two problems: (1) Matcher now checks all imports, and (2) matcher now supports fully-qualified method calls.
When reviewing my code changes, please focus on:
After the review, I'll merge this branch into:
master
Disallow lowercase l in long literals
Original issue created by [email protected] on 2012-07-16 at 05:50 PM
Representing a long literal with 5432L is far superior to 5432l for obvious reasons, and I think we should at least consider making the latter an error.
Check that @Override annotation is used in all appropriate places
Original issue created by [email protected] on 2012-08-28 at 09:42 PM
Check that any method that overrides another method is annotated with @Override. Perhaps this shouldn't be an error but rather a warning.
AST is missing comments
Original issue created by [email protected] on 2012-01-30 at 05:20 PM
When we scan an AST, the comments are missing. This prevents us checking file contents in comments, and also means that our suggested fix/refactoring loses comments in the tree being modified.
Code review request
Original issue created by [email protected] on 2011-10-21 at 05:58 PM
If you have some time, I've made some more commits which are unreviewed and I'd be interested in feedback.
Like we just discussed, there are still cases where the Exception reference escapes and this code reports a false positive error.
Run checks in Eclipse
Original issue created by [email protected] on 2012-01-29 at 07:56 PM
ECJ (eclipse compiler for Java) has its own representation of the source. We need to abstract our usage of the AST and symbol table to be able to operate on this representation. Then, we need to figure out where to wire in the execution of our TreeScanners.
Code review request
Original issue created by [email protected] on 2012-08-29 at 03:39 PM
Better suggested fix and test case for the Ordering.from refactoring:
Ordering.from(new Comparator<T>() { ... })
to
new Ordering<T>() { ... }
There might be a nicer way of getting hold of the "..." than the way I've found (which is basically to re-construct the AST object and pretty-print it): hints welcome.
GWT SafeHtml check
Original issue created by [email protected] on 2012-07-19 at 08:35 PM
GWT has an API called "SafeHtml" that has certain documented restrictions on how you're supposed to use it, and assuming developers use it correctly they should be largely protected against XSS vulnerabilities. Using it correctly amounts to two requirements:
- Some methods like SafeHtmlUtils.fromSafeConstant(String) are documented as requiring a "safe" string literal as the argument. Safe is defined here as a string that correctly parses as a sequence of complete HTML tags and leaves the parser in standard HTML context. (E.g., "" is okay; "<a href='" and "<script>" are not.)
- GWT has legacy methods like Element.setInnerHTML(String) that predate the introduction of SafeHtml APIs and subvert the protections offered, so they should be avoided in favor of the SafeHtml-variants.
My FindBugs works by recognizing the following expressions as safe:
- String literals that return true for com.google.gwt.safehtml.shared.SafeHtmlHostedModeUtils.isCompleHtml(String).
- String values returned from a call to SafeHtml.asString().
- A concatenation of string expressions recognized as safe (e.g., " + safeHtmlValue.asString() + "" is still safe).
It then warns about calls to methods like SafeHtmlUtils.fromSafeConstant(String) or Element.setInnerHTML(String) that aren't using safe arguments. Recognizing "safe" expressions this way is intentionally lenient to avoid needless code churn due to obviously safe (and incredibly common) code constructs like element.setInnerHTML("").
(It's complicated somewhat further because there are some methods that have an interface like setTextOrHTML(String text, boolean isHtml), and I further only warn on these methods if I can't statically determine the isHtml boolean argument is always false.)
Wiki generator includes .class files in wiki pages
Original issue created by [email protected] on 2012-01-28 at 01:18 AM
The wiki generator includes .class files in the generated wiki pages. It should only include .java files.
Fallthrough suppression shouldn't be on by default
Original issue created by [email protected] on 2012-05-24 at 04:29 PM
Tried turning on error prone in the Guava maven build.
Get lots of errors like:
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:2.3.2:compile (default-compile) on project guava: Compilation failure: Compilation failure:
[ERROR] /Users/alexeagle/guava-libraries/guava/src/com/google/common/math/LongMath.java:[74,3] [FallthroughSuppression] Fallthrough warning suppression has no effect if warning is suppressed
[ERROR](see https://github.com/cushon/issues-import/wiki/FallthroughSuppression)
[ERROR] did you mean to remove this line?
Add PreconditionsTooManyArgs bug pattern
Original issue created by [email protected] on 2012-10-26 at 10:47 PM
Branch name: preconditions-toomanyargs
Purpose of code changes on this branch:
Add a new bug pattern to match (especially) cases in which the wrong Precondition formatting placeholders have been used.
When reviewing my code changes, please focus on:
I have no idea what I'm doing.
After the review, I'll merge this branch into:
/trunk
Release 0.9 has bad tools jar dependency
Original issue created by [email protected] on 2012-05-30 at 04:31 AM
The maven release automatically resolved a variable from my local path, which isn't valid outside of my machine.
The current 0.9 release has this in the POM:
<dependencies>
<dependency>
<groupId>openjdk</groupId>
<artifactId>tools</artifactId>
<version>1.6</version>
system
<systemPath>/usr/local/buildtools/java/jdk6-google-v4/jre/../lib/tools.jar</systemPath>
</dependency>
Support Apple-provided JDK
Original issue created by [email protected] on 2012-05-21 at 03:22 AM
Currently, the way we hook into javac has only been tested with OpenJDK 6 and 7. Not sure how many developers expect to compile with Apple's JDK, since it is probably going to be sunset and Oracle will start maintaining the Mac version.
Consider supporting JSR 305 annotations
Original issue created by aftandilian on 2012-09-20 at 05:44 PM
JSR 305 is abandoned but proposes several new annotations to help static analysis tools. Presumably anyone using these annotations would like to have a tool check for correctness. FindBugs and IntelliJ already do this.
http://jsr-305.googlecode.com/svn/trunk/javadoc/javax/annotation/package-summary.html
Doesn't work on OpenJDK
Original issue created by [email protected] on 2012-01-30 at 05:19 PM
Our compile is broken against OpenJDK right now.
Additionally, the javac.util.Messages API in jdk6 has become javac.util.JavacMessages in jdk7, so we'll need to compile correctly against either version.
Invoking a static method from an instance
Original issue created by [email protected] on 2012-08-03 at 03:52 PM
someFoo.someStaticMethod() is a piece of awfulness that leads nowhere good. I think many teams would love to just make it an error.
errorMessageTemplates in Guava Preconditions class can only accept "%s" format variables
Original issue created by [email protected] on 2012-10-24 at 06:06 PM
Several methods in the Guava Preconditions class take an errorMessageTemplate that may be customized by substituting for "%s" in the template. Often the templates actually include other format specifiers ("%d", "%f", etc.), but those are not supported by the formatter.
Example:
Preconditions.checkArgument(i > 0, "i (%d) must be greater than zero", i);
should be
Preconditions.checkArgument(i > 0, "i (%s) must be greater than zero", i);
private Fields which should not be reassigned, but cannot be marked with final keyword
Original issue created by [email protected] on 2012-07-30 at 05:15 PM
Suggested by David Mankin:
In Guice, we have @Inject fields, which should only be set by the injector when creating the class. Since they can't be marked final, we often prefer to use constructor injection instead, even though it's quite bulky.
Similarly in GWT, fields cannot be marked final because the serialization mechanism needs to be able to set the values in the server when populating a data object.
We should be able to enforce that such fields are never re-assigned in code within the class.
We might want an annotation for this, maybe in JSR305, or maybe JSR330 (which would be nice since other DI frameworks should take advantage). For Guice, you could imagine @Inject(finalish=true) but if we do GWT, may as well make an annotation.
Probably not practical to do this with non-private fields since we'd have to scan outside the enclosing class.
Disallow annotating a non-static method with @BeforeClass
Original issue created by [email protected] on 2012-07-16 at 07:18 PM
The JUnit @BeforeClass annotation should only be applied to a public static void no-arg method, otherwise it throws a runtime error. We should detect this at compile time.
There may also be other JUnit-specific checks we can write. For example, JUnit doesn't run tests that are not marked public. This can cause someone to mistakenly think that their newly created tests are runnign when they're actually not. Some other reason tests weren't being run:
- The test class did not extend TestCase
- The test method wasn't public
- The test method didn't start with "test" (e.g. tesSomething)
- The test method took a parameter
Disallow non-static methods to be annotated org.junit.BeforeClass
Original issue created by [email protected] on 2012-07-16 at 06:48 PM
The BeforeClass method is called reflectively by JUnit before creating instances of the test class. So a non-static implementation causes an error.
Code review request
Original issue created by [email protected] on 2012-09-07 at 10:22 PM
Purpose of code changes on this branch:
Provide a way for matchers to request an arbitrary type. This is a fix for Issue 30: #30
Matchers set to severity level WARNING still produce hard errors
Original issue created by [email protected] on 2012-07-25 at 12:23 AM
JavacErrorDescriptionListener unconditionally calls log.error() regardless of the matcher's severity level. I think it should call log.warning() if the severity level is set to WARNING.
Code review request
Original issue created by [email protected] on 2011-09-26 at 11:56 PM
Branch name:
importfix
Purpose of code changes on this branch:
Fixed a failing test case where the Preconditions.checkNotNull() method being called was a different one from the one the check should match. Fixed this by altering the system to process import statements.
After the review, I'll merge this branch into:
master
EmptyIf diagnostic places caret at different AST node than javac
Original issue created by [email protected] on 2012-05-30 at 05:01 PM
CheckpointCopier.java:91: [empty] empty statement after if
if(true);
^
CheckpointCopier.java:91: [EmptyIf] Empty statement after if
if(true);
^
2 errors
Investigate operator precedence check
Original issue created by [email protected] on 2012-08-03 at 10:16 PM
Consider making unparenthesized &&-|| combinations into an error.
Code review request
Original issue created by [email protected] on 2011-11-04 at 02:16 PM
Purpose of code changes on this branch:
When reviewing my code changes, please focus on:
After the review, I'll merge this branch into:
/trunk
ReturnValueIgnored checker should support javax.annotation.CheckReturnValue annotation
Original issue created by aftandilian on 2012-09-20 at 05:42 PM
Our ReturnValueIgnored checker should support the javax.annotation.CheckReturnValue annotation from JSR 305.
Check that @Override annotation is used in all appropriate places
Original issue created by [email protected] on 2012-08-28 at 09:42 PM
Check that any method that overrides another method is annotated with @Override. Perhaps this shouldn't be an error but rather a warning.
Code review request
Original issue created by [email protected] on 2011-10-16 at 03:02 AM
Please review recent unreviewed changes, when you have a few minutes.
No particular standards yet. Thanks!
Calling getAnnotation on an annotation type that does not have runtime retention
Original issue created by [email protected] on 2012-08-29 at 12:46 AM
The Class.getAnnotation method may be called with an annotation type that is not retained a runtime, and thus it will always return null. error-prone should detect this and issue an error.
Filler for Non-Existent Issue
Filler for non-existent Google Code issue 21.
This issue only exists to ensure that GitHub issues have the same IDs they had on Google Code. Please ignore it.
Types generation from strings.
Original issue created by [email protected] on 2012-07-27 at 11:14 AM
As far as I dug into the error-prone and javac there is no convenient way to get a Type from String. And sometimes you need to check if the result type is a subtype of/castable to java.lang.Object[] or java.util.List<Integer>, etc.
Of course for plain classes (e.g. java.lang.Object) you can make a look-up in symtab, but you can't do the same for array types and parametrized classes. (Am I wrong here?)
I think it would be great to have such a method in error-prone library.
Something like "Type getType(String typeName)".
Code review request
Original issue created by [email protected] on 2012-07-21 at 12:30 AM
Refactored tests to make them more consistent and to share common code via a helper class.
Code review request
Original issue created by [email protected] on 2012-10-18 at 09:42 AM
Branch name: unneeded-ternary
Purpose of code changes on this branch:
Add new check for an unnecessary conditional operator
After the review, I'll merge this branch into:
/trunk
Code review request
Original issue created by [email protected] on 2011-11-30 at 04:40 PM
Purpose of code changes:
Added a empty-if-statements checker
When reviewing my code changes, please focus on:
Did I construct the new matchers correctly? Some of the type signatures were confusing. Everything does work.
Code review request
Original issue created by [email protected] on 2011-10-28 at 09:26 AM
Purpose of code changes on this branch:
Checking and fixing String.format() calls in Guava Preconditions calls.
When reviewing my code changes, please focus on:
General code review - I'm a beginner to this code, so I'm probably doing things in a non-optimal way.
After the review, I'll merge this branch into:
master
Code review request
Original issue created by [email protected] on 2012-03-24 at 12:45 AM
Added support to SuggestedFix to add or remove imports. Please take a look and let me know what you think.
Printed error messages are sometimes incorrect
Original issue created by [email protected] on 2012-01-10 at 12:55 AM
When a fix is to delete a line, but the line is not the same one where the error was found, the error message is incorrect. For example, this code:
if (i == 10)
;
{
foo...
}
Will produce the following error message:
[Empty if] Empty statement after if; did you mean to remove this line?
if (i == 10)
^
When you intend it to delete the line with the semicolon.
I've added a test case in emptyifstatement/PositiveCases.java under test5().
Code review request
Original issue created by [email protected] on 2012-10-01 at 10:34 PM
I fixed a bug with the compiler in the case that we are trying to scan a compilation unit that hasn't had all of its classes attributed yet.
Previously I had assumed that flow() would be called once on each class; that turns out not to be the case. We now track which classes error-prone has seen, and we only scan a compilation unit when we've seen all of its contained classes.
Code review request
Original issue created by [email protected] on 2012-07-31 at 10:39 PM
Please take a look at my commits from today (7/31).
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.