PMD 6.0.0#2771
Merged
Merged
Conversation
Move @SuppressWarnings closer to the problem to avoid suppressing future similar issues. Reduce amount of C-style (declare first and then use) variable definitions Make some variables final in tricky methods (enabled by above). Remove some unnecessary PMD suppressions Move PMD.OneDeclarationPerLine suppression for for loops into pmd config Small method extractions towards Clean Code Minor formatting/interface-impl enhancements in touched classes
…d#817) Fix hidden PMD violations
TWiStErRob
commented
Dec 31, 2017
| @@ -368,15 +368,15 @@ public synchronized void reset() throws IOException { | |||
| */ | |||
| @Override | |||
| public synchronized long skip(long byteCount) throws IOException { | |||
Collaborator
Author
There was a problem hiding this comment.
swapped order to mimic java.io.InputStream#skip (i.e. don't throw when no bytes are skipped)
TWiStErRob
commented
Dec 31, 2017
| @@ -66,37 +68,43 @@ public int getOrientation(Uri uri) { | |||
| } | |||
|
|
|||
| public InputStream open(Uri uri) throws FileNotFoundException { | |||
Collaborator
Author
There was a problem hiding this comment.
extracted cursor handling to separate method and move up validation to look like guards
TWiStErRob
commented
Dec 31, 2017
| String values = buildHeaderValue(entry.getValue()); | ||
| if (!TextUtils.isEmpty(values)) { | ||
| combinedHeaders.put(entry.getKey(), sb.toString()); | ||
| combinedHeaders.put(entry.getKey(), values); |
Collaborator
Author
There was a problem hiding this comment.
extracted method to simplify code, and remove duplicate sb.toString building.
TWiStErRob
commented
Dec 31, 2017
| int firstIfdOffset = segmentData.getInt32(headerOffsetSize + 4) + headerOffsetSize; | ||
| int tagCount = segmentData.getInt16(firstIfdOffset); | ||
|
|
||
| int tagOffset, tagType, formatCode, componentCount; |
Collaborator
Author
There was a problem hiding this comment.
didn't just split them up, but moved them close to usage, thus can be final
Collaborator
|
LGTM, thanks for cleaning this up! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I suggest a Squash Merge again. I suggest you take a look at the two revert commits and then the extra commits I added later. Please make sure you review thoroughly, hopefully rebase didn't mess anything up. Feel free to commit any amendments to the branch (you have write rights to all PR branches).
Description
Upgrade PMD from 5.8.1 to 6.0.0 based on https://github.com/sjudd/glide/tree/pmd_6_0_0 branch rebased onto #2762. Mostly PMD triggered fixes and refactors.
Motivation and Context
Better checks for finding hidden issues in the code.