onflow/cadence-tools

[Lint] Double force casting

Opened this issue · 7 comments

Current Behavior

Right now if you double force cast, for example, test["testing"]!!, the linter will tell you that you are performing an unecessary force unwrap.

However, given the fact that it's possible for dictionaries to have nil values, this actually may be needed some times to convert, for example, a String?? to a String

Expected Behavior

The linter should not complain about a double force cast if the type is a double optional

Steps To Reproduce

pub fun main(): AnyStruct {
  let bin: {String: String} = { "gg": "10" }
  var v : {String: AnyStruct} =  { "deniz": bin["gg"] }
  let thing = v["deniz"]!!.getType()
  return thing
}

Environment

- Cadence version: 0.40.0
- Network: Emulator (or any)

Doesn't this work as expected:

  • The value type of v is AnyStruct
  • v["deniz"] results in AnyStruct?
  • Two force-unwraps are used, but only one is necessary to unwrap

I think problem is: AnyStruct covering optional types. This is working as intended ( warning ) for sure.

But I think this should be only allowed to written as :

pub fun main(): AnyStruct {
  let bin: {String: String} = { "gg": "10" }
  var v : {String: AnyStruct?} =  { "deniz": bin["gg"] }
  let thing = v["deniz"]!!.getType()
  return thing
}

But as AnyStruct covers AnyStruct? this is going into area of cannot be statically determined.

Yeah, that's what I tried to point out: The analysis works only on static types, and from that perspective the implementation is correct.

However, you're right that AnyStruct as the top-type covers any other dynamic type, so the dynamic type of the underlying value could actually be optional. This is unlike any other type (e.g. think through the example with the dictionary's value type being e.g. Int).

So the analysis could be improved here to ignore Any/AnyStruct/AnyResouce, to avoid reporting a warning that the operator is unnecessary, where it might actually be necessary. 👍

In particular, the exception should be added here:

_, ok = valueType.(*sema.OptionalType)
if !ok {

However, you're right that AnyStruct as the top-type covers any other dynamic type, so the dynamic type of the underlying value could actually be optional. This is unlike any other type (e.g. think through the example with the dictionary's value type being e.g. Int).

Btw is it possible to change hierarchy, like putting Optional higher than AnyStruct? This maybe totally stupid idea. I have no idea how other languages work, or theory of this stuff as I don't have any CS background.

Currently AnyStruct and AnyStruct? seems like same type. (in a sense)

is it possible to change hierarchy, like putting Optional higher than AnyStruct?

No, AnyStruct is the top type (for values / non-resources). A good analogy is thinking of optionals as "zero-or-one containers". Moving them up in the hierarchy makes not much sense, just like moving up arrays ("zero-or-more containers") makes not much sense.

These are great questions, and I am happy to answer, but let's move such questions to the forum, they are unrelated to the issue.