[SourceKit] Determine correctly whether typed variables in if/guard/while-let-statements have explicit type annotations

[fwcd]

As noted in swiftlang/sourcekit-lsp#408 (comment), if/guard/while-lets are currently (incorrectly) always reported as having no explicit type annotation as the TypedPattern is not the top-level-pattern in such declarations. Consider the following example:

if let x: Int = Optional.some(4) {}

Here we get the following AST:

(if_stmt ...
  (pattern
    (pattern_optional_some implicit type='Int?'  <--- this node prevents the current mechanism from discovering the type annotation
      (pattern_typed type='Int'
        (pattern_let implicit type='Int'
          (pattern_named type='Int' 'x'))
        (type_ident
          (component id='Int' bind=Swift.(file).Int))))
    (call_expr ...))
  (brace_stmt ...))

sourcekitd-test therefore reports (explicit type: 0) despite an explicit annotation being present:

<VariableTypes>
(1:8, 1:9): Int (explicit type: 0)
</VariableTypes>

This PR fixes the issue by including an implicitly generated OptionalSomePattern as a special case in VariableTypeCollector and also adds tests to verify that these cases are handled correctly.

cc @ahoppen

[ahoppen]

I think it might be worth adding the check to getTypeReprOrParentPatternTypeRepr. It seems generic enough to me that other user’s of the API could also benefit from it.

Or was there a specific reason why you didn’t do that?

[fwcd]

It would change the semantics of the method slightly (by not just considering the parent pattern), therefore I wasn't sure whether this should be included.

[ahoppen]

I think changing the semantics of getTypeReprOrParentPatternTypeRepr is fine if it doesn’t break any tests – I would consider it an improvement to that method.

I would also leave the implicitness check unless there’s a specific reason to include it that you can explain. Just makes the code simpler and more generic.

[ahoppen]

@swift-ci Please smoke test

[fwcd]

Not entirely sure about the CI failure, but it seems unrelated to the PR?

[ahoppen]

Yes, the CI failure is unrelated. I’ll try again later.

[ahoppen]

@swift-ci Please smoke test Linux

[ahoppen]

@swift-ci Please smoke test Linux