Implicit Date to Datetime conversion inspection

Issue #1009 resolved
Phil W created an issue

It would be very useful to have an Apex inspection that can identify all usages of implicit date to date time conversion because this fails to consider time zones (always resulting in a value based on UTC/GMT). An implicit conversion is applied when assigning a Date to a Datetime variable or when there is an implicit or explicit requirement for a cast from Date to Datetime.

The explicit conversion equivalent, typically of the form Datetime.newInstance(theDate, Time.newInstance(0,0,0,0)), does address the time zone.

Comments (11)

  1. Scott Wells repo owner

    So just so I’m clear about what’s being requested, effectively anytime that IC2 sees a Date-type expression being assigned to a Datetime-type variable – which could be a field, a local variable, a constructor/method formal parameter, etc., IC2 should flag the Date-type expression stating that the resulting Datetime may not be as-expected and offer an intention to convert the Date-type expression into Datetime.newInstance(<dateTypeExpression>, Time.newInstance(0, 0, 0, 0)), correct?

    To provide some concrete examples, in the following:

    Date d = Date.today();
    Datetime dt = d;
    

    the expression d on the righthand side of the assignment expression would be flagged as a warning, and if the user triggers the intention, the last statement would be changed to:

    Datetime dt = Datetime.newInstance(d, Time.newInstance(0, 0, 0, 0));
    

    And in the following:

    String sobjectType = Account.SObjectType.getDescribe().getName();
    Date startDate = ...;
    Date endDate = ...;
    Database.GetUpdatedResult result = Database.getUpdated(
        sobjectType,
        startDate, 
        endDate
    );
    

    the invocation argument expressions startDate and endDate would be flagged as warnings, and application of the intention to both would change the last statement to:

    Database.GetUpdatedResult result = Database.getUpdated(
        sobjectType, 
        Datetime.newInstance(startDate, Time.newInstance(0, 0, 0, 0)),
        Datetime.newInstance(endDate, Time.newInstance(0, 0, 0, 0))
    );
    

    Is that a correct understanding of the requested behavior?

  2. Phil W reporter

    The above is what I would want for assignment cases. The other area of problem is in logical conditions, and specifically comparisons. For example,

    Date startDate = ...;
    Datetime startDatetime = ...;
    if (startDatetime >= startDate) {
        ...
    }
    

    should be flagged as problematic, with the solution being along the lines you have shown for “promoting” a Date to a Datetime with midnight in the user’s time zone. I.e. something like:

    Date startDate = ...;
    Datetime startDatetime = ...;
    if (startDatetime >= Datetime.newInstance(startDate, Time.newInstance(0, 0, 0, 0)) {
        ...
    }
    

  3. Scott Wells repo owner

    Thanks for the clarification. At this point this won’t make it into this week’s build, but I’ll definitely take a look for next week’s.

  4. Scott Wells repo owner

    This has now been implemented as an enhancement to the existing Apex Illegal Assignment code inspection, and I’m running regression tests accordingly. Here’s an example of the inspection itself:

    and the associated intention/quick fix which includes special handling for Date.today() => Datetime.now() and System.today() => System.now():

    Here is the offending code before applying the quick fix:

    Date today = Date.today();
    Datetime now = Datetime.now();
    
    Datetime dt2 = Date.today();
    Datetime dt3 = today;
    
    if (now == today) {}
    if (today != now) {}
    if (Datetime.now() < Date.today()) {}
    if (Date.today() > Datetime.now()) {}
    
    takesDatetime(today);
    

    and here it is after applying the quick fix to all instances of the issue:

    Date today = Date.today();
    Datetime now = Datetime.now();
    
    Datetime dt2 = Datetime.now();
    Datetime dt3 = Datetime.newInstance(today, Time.newInstance(0, 0, 0, 0));
    
    if (now == Datetime.newInstance(today, Time.newInstance(0, 0, 0, 0))) {}
    if (Datetime.newInstance(today, Time.newInstance(0, 0, 0, 0)) != now) {}
    if (Datetime.now() < Datetime.now()) {}
    if (Datetime.now() > Datetime.now()) {}
    
    takesDatetime(Datetime.newInstance(today, Time.newInstance(0, 0, 0, 0)));
    

    Please let me know if this is not a correct interpretation of the request and/or if the inspection/quick fix wordings don’t make good sense.

  5. Phil W reporter

    Sorry, I missed this message in the forest of emails from our internal systems.

    I’m not sure the text is quite right since it can actually change the date as well as the time depending on the time zone for the user.

    Instead of:

    Implicit conversion of ‘Date’ to ‘Datetime’ can result in a ‘Time’ component with an incorrect or unexpected time zone.

    I suggest:

    Implicit conversion of ‘Date’ to ‘Datetime’ results in a ‘Datetime’ in GMT time zone, which may not be expected or appropriate.

    Otherwise looks good.

  6. Scott Wells repo owner

    Thanks for the feedback. I ended up going with “Implicit conversion of Date to Datetime uses GMT as the time zone which may result in an incorrect or unexpected final value.”

  7. Log in to comment