Skip to content

Conversation

@mpritham
Copy link
Contributor

Before this PR

We had a refaster template which suggested that {implementation of Collection}.size() == 0 be changed to {implementation of Collection}.isEmpty(). This was removed in this PR.

After this PR

==COMMIT_MSG==
Add check: Use Collection.isEmpty() instead of comparing size() with 0
==COMMIT_MSG==

@mpritham mpritham self-assigned this Mar 24, 2023
@changelog-app
Copy link

changelog-app bot commented Mar 24, 2023

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Add check: Use Collection.isEmpty() instead of comparing size() with 0

Check the box to generate changelog(s)

  • Generate changelog entry

Pritham Marupaka added 3 commits March 30, 2023 17:25
…ub.com:palantir/gradle-baseline into migrate-refaster-templates-to-errorprone-rules

private static boolean isExpressionThis(ExpressionTree tree) {
switch (tree.getKind()) {
case IDENTIFIER:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the memberselect case is technically necessary here for "qualified this" usages e.g.

class Outer {
  int foo();
  
  Inner getInner() {
    return new Inner();
  }
  class Inner {
    int foo() {
        // 'return foo()' would recurse infinitely
        // this allows us to reference components on an enclosing class from a non-static inner class.
        return Outer.this.foo();
    }
  }
}

Copy link
Contributor Author

@mpritham mpritham Mar 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. Have not seen this before. Updated, and added a test.

Copy link
Contributor

@carterkozak carterkozak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great tests!

@bulldozer-bot bulldozer-bot bot merged commit 11ef5b5 into develop Mar 31, 2023
@bulldozer-bot bulldozer-bot bot deleted the migrate-refaster-templates-to-errorprone-rules branch March 31, 2023 18:38
@svc-autorelease
Copy link
Collaborator

Released 5.2.0

This was referenced Mar 31, 2023
bulldozer-bot bot pushed a commit to palantir/witchcraft-api that referenced this pull request Apr 1, 2023
###### _excavator_ is a bot for automating changes across repositories.

Changes produced by the roomba/latest-baseline-oss check.

# Release Notes
## 5.2.0
| Type | Description | Link |
| ---- | ----------- | ---- |
| Improvement | Add check: Use Collection.isEmpty() instead of comparing size() with 0 | palantir/gradle-baseline#2530 |



To enable or disable this check, please contact the maintainers of Excavator.

EqualsZeroExpression equalsZeroExpression = maybeEqualsZeroExpression.get();
ExpressionTree operand = equalsZeroExpression.operand();
ExpressionTree collectionInstance = ASTHelpers.getReceiver(operand);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #2543

PR palantir/tritium#1688 is failing attempting to upgrade to gradle-baseline with this change on https://github.com/palantir/tritium/blob/13799047a8b715b4ebf395de80deed78a86b734c/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/TagMap.java#L120-L123

> Task :tritium-registry:compileJava FAILED
/home/circleci/project/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/TagMap.java:123: error: An unhandled exception was thrown by the Error Prone static analysis plugin.
            if (comparisonResult == 0) {
                                 ^
     Please report this at https://github.com/google/error-prone/issues/new and include the following:
  
     error-prone version: 2.18.0
     BugPattern: CardinalityEqualsZero
     Stack Trace:
     java.lang.IllegalStateException: Expected expression 'comparisonResult' to be a method invocation or field access, but was IDENTIFIER
  	at com.google.errorprone.util.ASTHelpers.getReceiver(ASTHelpers.java:605)
  	at com.palantir.baseline.errorprone.CardinalityEqualsZero.matchBinary(CardinalityEqualsZero.java:61)
  	at com.google.errorprone.scanner.ErrorProneScanner.processMatchers(ErrorProneScanner.java:449)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitBinary(ErrorProneScanner.java:512)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitBinary(ErrorProneScanner.java:150)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants