JetBrains
[Range] and [Min] attributes are considered hard limits
Consider the following snippet:
using UnityEngine;
public class RiderTest : MonoBehaviour {
[Min(1)] public int SmallNumber = 1;
[Range(1, 2)] public int BigNumber = 1;
void Start() {
SmallNumber -= 1;
if (SmallNumber <= 0) Debug.Log("this limit only applies in the editor!");
BigNumber += 1;
if (BigNumber <= 0) Debug.Log("this limit only applies in the editor!");
if (BigNumber > 2) Debug.Log("this limit only applies in the editor!");
}
}
As the comparison lies outside the [Min] and [Range] attributes Rider will flag the conditionals as "Expression is always false", however, those limitations only apply in the editor. Not for runtime code.
This information is useful, but should perhaps not quite be a warning.
Why would the ranges be different in the editor and at runtime? What's the point of setting a range that's not valid?
Sorry, the "editor" distinction wasn't particularly clear. The limits only apply in the editor UI. There is no limit preventing the field to be set to whatever via code.
Technically, no. You can change them to anything, and Rider will do its best to update what it knows about the value. For example, if you try to do BigNumber = 10 in your example above, then Rider will show a warning that this is outside of the range that you're saying is unacceptable. But it can't catch everything - BigNumber = UnknownValue won't show a warning, because Rider doesn't know what UnknownValue is. Similarly, BigNumber += 1 is fine, because 1+1 is still within range, even though 2+1 puts it outside. But Rider would warn on BigNumber += 2.
The idea is that these annotations are hints to analysis that the fields have an acceptable set of ranges, and Rider will warn if you do something that it can tell is outside of this range. It doesn't prevent you from making the changes, but it tries to give detail where it can. You can alt+enter and set severity of these highlights, in the "Inspection: …" sub menu. By default, they're set as warning, but you can change this to suggestion, or a less intrusive hint, or even disable if you wish.
The above snippet triggers a warning for me checking if the value is outside of the range I knowingly specified. This makes little sense to me. It even says "Expression is always false" which simply isn't true.
The warning for directly setting a value outside the range makes more sense to me, that's also clearer about the problem ("Possible violation of ..."), I don't oppose that.
This assumption would make sense in an Editor UI only setting, but that's not why I'm using Rider, I'm writing code.
Disabling/downgrading the warning would be for the entire class of "Expression is always false"-inspections though, which isn't a tradeoff that makes much sense to me.
This is because the attributes are specifying an invariant about the value. We're saying SmallNumber will always have a minimum value of 1, in the same way that [NotNull] says that the value will never be null, and so if (foo == null) checks are redundant.
The inspection on the if looks wrong because we have outside knowledge of the data flow of the app, that Rider doesn't have. We know that the serialised field isn't modified by any other code before Start is called (and we know that Start isn't called multiple times), so we know that SmallNumber is 1 at this point, and subtracting one will break our invariant.
Rider can't do this level of data flow, so doesn't know this. It can't add a warning to the subtraction (because it would be too noisy - you'd get a warning for all arithmetic), so all it can do is assume that the invariant still holds, and that the value will always be at least 1.
But we are breaking the invariant here. Arguably, we should be writing the subtraction as SmallNumber = Math.Min(1, SmallNumber - 1). This way, we know the invariant still holds and the if statements are genuinely redundant.
At least, that's the theory 😄
In practice, I'm less convinced. I get that Rider doesn't and can't know the value of SmallNumber when Start is called. And I get that we're trying to set up an invariant to see that the value will always be between a certain range. But I agree that the inspection on the if statement doesn't seem right - if it takes this much explaining, then it's not exactly intuitive.
I would like Rider to be able to warn me that the subtraction is dangerous, because I could very easily go out of bounds (and it does this nicely with simple examples, like [Range(10, 20)] and SmallNumber += 100) but I also get that this would lead to a warning on all arithmetic expressions, which would be pointless.
I'm not sure what the answer is for this inspection. I'll raise it internally to see what the thoughts are about the unintuitive warning, and especially the fact that you can't disable the inspection for numerical analysis, while leaving it active for other analysis.
And I think it's worth adding an option to disable this analysis for Unity attributes.
I fully respect that this is Hard™!
I guess I always considered those attributes as mostly guidance for the Editor, but viewing them as more important than that is valid too. Either way, my normal experience with Rider's hints is that they are correct and I messed up, which it didn't feel like this time hence this issue.
I'll look forward to whatever you decide is the best solution.