Invalid suggestion for psalm --alter and strange const error

[ricardoboss]

There's a lot to unpack here... Any idea why any of this is happening? I don't understand the error message (I have not declared any type for the const, it should infer the type from the value) and the --alter suggestion doesn't even work.

[psalm-github-bot]

Hey @ricardoboss, can you reproduce the issue on https://psalm.dev ?

[ricardoboss]

Yeah, I can reproduce it... but why? https://psalm.dev/r/d757d80ca9

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/d757d80ca9

<?php


interface Duration
{
	public const SECONDS_PER_MINUTE = 60;
	public const SECONDS_PER_HOUR = self::SECONDS_PER_MINUTE * self::MINUTES_PER_HOUR;
	public const SECONDS_PER_DAY = self::SECONDS_PER_HOUR * self::HOURS_PER_DAY;
    
    // THE LINE BELOW causes the error?
	public const SECONDS_PER_MONTH = self::SECONDS_PER_DAY * self::DAYS_PER_MONTH;
    
	public const MINUTES_PER_HOUR = 60;
	public const HOURS_PER_DAY = 24;
	public const DAYS_PER_MONTH = self::DAYS_PER_YEAR / self::MONTHS_PER_YEAR;
	public const DAYS_PER_YEAR = 365.2425; // average length of a Gregorian year over a complete leap cycle of 400 years
	public const MONTHS_PER_YEAR = 12;
}

Psalm output (using commit f960d71):

ERROR: InvalidConstantAssignmentValue - 15:15 - Duration::DAYS_PER_MONTH with declared type float(30.436875) cannot be assigned type float

[ricardoboss]

Also, if I add:

/**
 * @psalm-suppress InvalidConstantAssignmentValue
 */

to the class, it still reports the errors. Is this supposed to work? It also doesn't work if I add it to each individual const, reporting each as unused next to the error I want to suppress.

[orklah]

@AndrolGenhald That's weird, seems like the property is analyzed twice with two different values for the calculation

[AndrolGenhald]

Yeah, I've run into this before but most of the issues were fixed. I think the problem is that it's being resolved once by ClassLikeNodeScanner using SimpleTypeInferer to infer the type, then it's being resolved again when evaluating the assignment, and for some reason the assignment evaluation isn't as specific. I'll see if I can take a look at it this week.

[ricardoboss]

If it helps: the order matters!

Moving the DAYS_PER_MONTH const above all others doesn't produce the error.

[AndrolGenhald]

As does moving SECONDS_PER_MONTH to the bottom, looks like the analysis isn't working correctly when constants are used before they're declared.

[ricardoboss]

Apart from the consts, what about the psalm --alter suggestion? Is this just not implemented (yet)?

[AndrolGenhald]

I'm not sure what's going on there, I don't think that issue should even be showing up as fixable. I'll look into that as well.

[AndrolGenhald]

Turns out the issue is in BinaryOpAnalyzer, it calculates literal expressions for ints but not for floats. The constant type inferrer gets the literal type when determining the constant's type, but then when the statement is later analyzed to make sure the assignment matches the constant's type it doesn't calculate the literal type, so it doesn't match. Working on a fix.