False positive syntax error

Issue #2349 resolved
Adam Stepanek created an issue

Hi Scott,

we have the following code snippet in our code base, which is displaying as a syntax error, despite successful deployment.

public SKUStackingLogic(SObjectField productCode, SObjectField startDate, SObjectField endDate) {
    itemsToBeUpdated = new List<SObject>();
    this.productCode = productCode;
    this.startDate = startDate;
    this.endDate = endDate;
}

public SKUStackingLogic(SObjectType sObjectType) {
    if (sObjectType == Opportunity.SObjectType) {
        this(OpportunityLineItem.ProductCode, OpportunityLineItem.ServiceDate, OpportunityLineItem.Id);
    } else {
        throw new IllegalArgumentException(sObjectType + ' is not supported SObjectType');
    }
}

Comments (4)

  1. Scott Wells repo owner
    • changed status to open

    Interesting. Looks like it's incorrectly interpreting that as an SObject constructor call. I'll take care of it. Thanks for reporting.

  2. Scott Wells repo owner

    Okay, I figured this out. IC2’s Apex parser currently expects the explicit constructor invocation statement (super() or this()) to be the first line of the constructor body. Obviously that’s not required by the Apex language, though. Superficially it’s a very simple change to make, though with any parser change I want to run it through the paces. I’ll also need to add a static code analysis rule that verifies that 1) explicit constructor invocation statements are only found in constructors; 2) any given code path includes at most one explicit constructor invocation statement; and ideally 3) an explicit constructor invocation statement is present in all code paths when required.

    That’s more than will fit in the current release as it’s otherwise ready to go out tomorrow, but I’ll see if I can get this into next week’s release.

  3. Scott Wells repo owner

    Okay, I’ve checked in a fix for this with improved static code analysis around explicit constructor invocations as a bonus, but I confess that I’m surprised that this is valid…though it is. In fact, the error message you receive from Salesforce’s own compiler when you create various error scenarios around explicit constructor invocations is:

    Call to 'super()/this()' must be the first statement in a constructor method

    and in Java code like what you’ve demonstrated would result in a compilation error. My guess is that Salesforce is loosely interpreting it as the first executed leaf statement in the constructor body with the possibility of control flow statements leading to that statement as long as they’re not setting/changing instance state.

    Anyway, since it’s valid, IC2 will now accept it and even try to validate it, but since I don’t 100% know what logic Salesforce is using to determine “first statement in a constructor method”, it’s possible there could be some corner-case false negatives to its static code analysis for this behavior.

    The fix will be included in next week’s build, typically on Thursday morning.

  4. Log in to comment