Skip to content

Java: PrimitiveType.getADefaultValue() is misleading and might not work correctly #6615

@Marcono1234

Description

@Marcono1234
Contributor

There are several issues with PrimitiveType.getADefaultValue():

  • The documentation says:

    Gets a default value for this primitive type, as assigned by the compiler for variables that are declared but not initialized explicitly.

    This is misleading; unless the source actually contains literals with the default value, the predicate will have no result (except for some BooleanLiterals which are found in the JDK code as part of annotations). For example for this Java code:

    class DefaultLiteral {
        byte b;
        short s;
        int i;
        long l;
        float f;
        double d;
        boolean bool;
        char c;
        
        Object o;
    }

    the following query only finds BooleanLiterals:

    import java
    
    from PrimitiveType t, Literal defaultValue
    where
    defaultValue = t.getADefaultValue()
    select t, defaultValue, defaultValue.getLocation()
  • The way it matches literals by using getLiteral() misses some cases where getValue() would actually represent the default value (relates to Java: Replace incorrect usage of Literal.getLiteral() #6612).

Would it make sense to deprecate the predicate (for removal) and instead adjust the only query using it (DeadStoreOfLocal.ql)?
It appears that query actually wants to find out if an assigned literal has a default value; so maybe it should be rewritten to explicitly test that. (Note that adding a predicate Literal.hasDefaultValue() might not make much sense because technically for StringLiterals type String the default value is null; and NullLiteral has no type which could occur as field type.)

Activity

smowton

smowton commented on Sep 6, 2021

@smowton
Contributor

To avoid surprising existing users we could redocument this to note what it really does (enumerate actual literals in source code that so happen to match the default the compiler would have assigned for an expression of this type), and perhaps add a different predicate that gives a string rather than a Literal and therefore doesn't rely on having a real entity to point at for a source location.

aschackmull

aschackmull commented on Sep 7, 2021

@aschackmull
Contributor

Yeah, that predicate is indeed confusing to the point of near-uselessness.

Would it make sense to deprecate the predicate (for removal) and instead adjust the only query using it (DeadStoreOfLocal.ql)?
It appears that query actually wants to find out if an assigned literal has a default value; so maybe it should be rewritten to explicitly test that.

Yes, I'd be in favour of that solution.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    JavaacknowledgedGitHub staff acknowledges this issuequestionFurther information is requested

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      Participants

      @smowton@Marcono1234@aschackmull@hmakholm

      Issue actions

        Java: `PrimitiveType.getADefaultValue()` is misleading and might not work correctly · Issue #6615 · github/codeql