Invalid inspector highlight at Final Static property with get and set.

Issue #2436 resolved
Stanisław Zań created an issue

The following code is compiling and working correctly, but Inspector highlights it as an error:\

public static final Integer FINAL_SINGLETON {
    get {
        FINAL_SINGLETON = 1000000;
        return FINAL_SINGLETON;
    }
    private set;
}

Error:

Static final property ‘FINAL_SINGLETON’ can only be assigned a value in the declaration or a static initializer block. Lazy initialization is also allowed in ‘get’ as long as it is guaranteed to execute exactly once.

It was not highlighted as an error a few releases ago, I’d guess it started around 2.2.6.X maybe? I'm not sure about the version, but that code is about a year old and it was not highlighted then, it started to highlight recently.

Comments (6)

  1. Scott Wells repo owner

    Hi. Yes, you’re correct that final/immutability checks for properties were added recently, specifically in 2.2.8.1:

    https://illuminatedcloud.blogspot.com/2023/08/2281-release-notes.html

    Technically speaking, the code that you’ve presented should result in a FinalException as the property value is being assigned each time the property value is read, but evidently Apex isn’t actually enforcing final properly for properties. To prove that, I changed it slightly as follows:

    public with sharing class Issue2436 {
        public static final Double FINAL_SINGLETON {
            get {
                FINAL_SINGLETON = Math.random();
                return FINAL_SINGLETON;
            }
            private set;
        }
    
        public static void callGetTwice() {
            System.debug(FINAL_SINGLETON);
            System.debug(FINAL_SINGLETON);
        }
    }
    

    and then invoked that method:

    EinsteinGptDemo.callGetTwice();
    

    which resulted in the following:

    06:43:17.158 (168734951)|USER_DEBUG|[13]|DEBUG|0.835086845752237
    06:43:17.158 (168771415)|USER_DEBUG|[14]|DEBUG|0.7777391947652996
    

    As you can see, that “immutable” property is happily taking on multiple values.

    So I think that warning that unguarded assignment of the value in the read accessor is probably still a good thing because otherwise final doesn’t necessarily mean immutable, but of course in your example it’s being assigned to the exact same literal value each time. IC2 is already providing an allowance for a lazy-initialization pattern in property read accessors and wouldn’t complain about the following:

    public static final Integer FINAL_SINGLETON {
        get {
            if (FINAL_SINGLETON == null) {
                FINAL_SINGLETON = 1000000;
            }
            return FINAL_SINGLETON;
        }
        private set;
    }
    

    I’ll look at adding another allowance for a single, consistent assignment to a literal value.

    You can also suppress these specific instances of the inspection as follows:

    which will add an annotation like the following, thereby disabling the inspection for that specific property as follows:

        @SuppressWarnings('ApexControlFlowAnalysis')
        public static final Double FINAL_SINGLETON {
            get {
                FINAL_SINGLETON = Math.random();
                return FINAL_SINGLETON;
            }
            private set;
        }
    

    You can also suppress just that specific line using the comment-based option.

    I’ll keep this open to investigate another potential allowed pattern for property value initialization, though, but hopefully what I’ve shown above is evidence of why it’s reporting these types of things in general. The Apex compiler doesn’t report final violations consistently at compile-time, instead throwing a FinalException at runtime which is of course dangerous, and in the specific example I provided above, it actually doesn’t enforce the final modifier at all which could be surprising depending on how it’s used.

  2. Stanisław Zań reporter

    Oh my God yes, you are absolutely right that’s indeed an Apex error to allow reassigning final in that get block. I just blindly trusted the Salesforce compiler/interpreter that it enforces final "more properly" than the IDE addon.

    In that case, I’d say it’s better that the highlight is there actually.

  3. Scott Wells repo owner

    Okay, if you're in agreement that it's flagging something that requires closer inspection, I'm going to resolve this. If you convert that immediate assignment into a lazy initialization/assignment (since Apex doesn't have issues with concurrency), IC2 won't flag it.

  4. Log in to comment