Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION false positive with super class #2050

Closed
rovarga opened this issue May 13, 2022 · 6 comments
Closed
Labels

Comments

@rovarga
Copy link
Contributor

rovarga commented May 13, 2022

With spotbugs-4.7.0, the following snipped incurs THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION:

import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;

    private static final LoadingCache<Object, SingletonSet<Object>> CACHE = CacheBuilder.newBuilder().weakValues()
            .build(new CacheLoader<Object, SingletonSet<Object>>() {
                @Override
                public SingletonSet<Object> load(final Object key) {
                    return SingletonSet.of(key);
                }
            });

The resulting bytecode for the anonymous CacheLoader subclass looks like this:

{
  org.opendaylight.yangtools.util.SharedSingletonMap$1();
    descriptor: ()V
    flags: (0x0000)
    Code:
      stack=1, locals=1, args_size=1
         0: aload_0
         1: invokespecial #1                  // Method com/google/common/cache/CacheLoader."<init>":()V
         4: return
      LineNumberTable:
        line 72: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       5     0  this   Lorg/opendaylight/yangtools/util/SharedSingletonMap$1;

  public org.opendaylight.yangtools.util.SingletonSet<java.lang.Object> load(java.lang.Object);
    descriptor: (Ljava/lang/Object;)Lorg/opendaylight/yangtools/util/SingletonSet;
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=1, locals=2, args_size=2
         0: aload_1
         1: invokestatic  #7                  // Method org/opendaylight/yangtools/util/SingletonSet.of:(Ljava/lang/Object;)Lorg/opendaylight/yangtools/util/SingletonSet;
         4: areturn
      LineNumberTable:
        line 75: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       5     0  this   Lorg/opendaylight/yangtools/util/SharedSingletonMap$1;
            0       5     1   key   Ljava/lang/Object;
    MethodParameters:
      Name                           Flags
      key                            final
    Signature: #27                          // (Ljava/lang/Object;)Lorg/opendaylight/yangtools/util/SingletonSet<Ljava/lang/Object;>;

  public java.lang.Object load(java.lang.Object) throws java.lang.Exception;
    descriptor: (Ljava/lang/Object;)Ljava/lang/Object;
    flags: (0x1041) ACC_PUBLIC, ACC_BRIDGE, ACC_SYNTHETIC
    Code:
      stack=2, locals=2, args_size=2
         0: aload_0
         1: aload_1
         2: invokevirtual #13                 // Method load:(Ljava/lang/Object;)Lorg/opendaylight/yangtools/util/SingletonSet;
         5: areturn
      LineNumberTable:
        line 72: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       6     0  this   Lorg/opendaylight/yangtools/util/SharedSingletonMap$1;
    Exceptions:
      throws java.lang.Exception
    MethodParameters:
      Name                           Flags
      key                            final synthetic
}

SpotBugs will complain about the second method and (based on line numbers) will point the user to the perfectly-fine looking source code.

@KengoTODA
Copy link
Member

@baloghadamsoftware could you check this issue when you have time? Thanks in advance!

@KengoTODA KengoTODA added the bug label May 14, 2022
@baloghadamsoftware
Copy link
Contributor

@oroszbd Could please take a look on this one?

@famod
Copy link

famod commented May 18, 2022

IMO, there should be no such violation in general when the exception declaration is inherited from a super class or interface.
While it's true that you can and probably should reduce the throws declaration to what can actually happen in the concrete impl, it all becomes very awkward when lambda expressions are involved (for which you cannot alter the declaration).

@iloveeclipse
Copy link
Member

IMO, there should be no such violation in general when the exception declaration is inherited form a super class or interface.

Exact. We see literally hundreds of warnings reported on implementors of ISafeRunnable which is defined like

	/**
	 * Runs this runnable.  Any exceptions thrown from this method will
	 * be logged by the caller and passed to this runnable's
	 * {@link #handleException} method.
	 *
	 * @exception Exception if a problem occurred while running this method
	 * @see SafeRunner#run(ISafeRunnable)
	 */
	public void run() throws Exception;

See
https://github.com/eclipse-equinox/equinox.bundles/blob/fd2f4fbc8f5be1ff93d197e974ffd3482788e837/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/ISafeRunnable.java#L38

@baloghadamsoftware
Copy link
Contributor

IMO, there should be no such violation in general when the exception declaration is inherited from a super class or interface.

I agree. The case where the exception declaration is inherited from the superclass must be ignored.

@iloveeclipse
Copy link
Member

Duplicate of #2040

@iloveeclipse iloveeclipse marked this as a duplicate of #2040 May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants