[AccessorMacro] 'let' declarations cannot be computed properties

[vanvoorden]

Description

The following Swift code fails to compile:

let x: Int {  //  'let' declarations cannot be computed properties
  1
}

It looks like our current AccessorMacro implementation adds computed getters to let variables without transforming the let to var:

extension AccessorMacroTests {
  func testLetDeclarations() {
    assertMacroExpansion(
      """
      @constantOne
      let x: Int = 1
      """,
      expandedSource: """
        var x: Int {
          get {
            return 1
          }
        }
        """,
      macros: ["constantOne": ConstantOneGetter.self],
      indentationWidth: indentationWidth
    )

    // Bit of an odd case, compiler has the type but we don't know it in `MacroSystem`
    assertMacroExpansion(
      """
      @constantOne
      let x = 1
      """,
      expandedSource: """
        var x {
          get {
            return 1
          }
        }
        """,
      macros: ["constantOne": ConstantOneGetter.self],
      indentationWidth: indentationWidth
    )
  }
}

This test fails:

error: -[SwiftSyntaxMacroExpansionTest.AccessorMacroTests testLetDeclarations] : failed - Macro expansion did not produce the expected expanded source
–var x: Int {
+let x: Int {
   get {
     return 1
   }
 }

Actual expanded source:

let x: Int {
  get {
    return 1
  }
}
error: -[SwiftSyntaxMacroExpansionTest.AccessorMacroTests testLetDeclarations] : failed - Macro expansion did not produce the expected expanded source
–var x {
+let x {
   get {
     return 1
   }
 }

Actual expanded source:

let x {
  get {
    return 1
  }
}

Should we update the AccessorMacro implementation to produce Swift more consistent with what engineers are able to compile? Should we be transforming that let to a var?

Steps to Reproduce

No response

[ahoppen]

Tracked in Apple’s issue tracker as rdar://122690037

[Matejkob]

I've investigated how such an accessor macro will behave in the compiler and it seems that accessor macro can bypass 'let' declarations cannot be computed properties requirement.

Such a code:

struct A {
  @constantOne()
  let x: Int = 0
}

let a = A()

print(a.x)

compiles with no errors and it prints 1.

[ahoppen]

I thought I had left a comment here but apparently didn’t. I think that we should disallow accessor macros on let variables inside the type checker.

[vanvoorden]

I think that we should disallow accessor macros on let variables inside the type checker.

@ahoppen Hmm… I was actually expecting that I might have a use case in a future example.

Suppose I created some kind of Storage macro that works on a struct to synthesize getters and (when needed) setters on some kind of internal Storage class:

@Storage struct Person {
  let id: String
  var name: String
}

This might then expand to something like:

struct Person {
  var id: String {
    get {
      self.storage.id
    }
  }
  var name: String {
    get {
      self.storage.name
    }
    set {
      self.storage.name = newValue
    }
  }
  
  private final class Storage {
    let id: String
    var name: String
    
    //  TODO: INIT...
  }
  
  private var storage = Storage()
}

Obviously there would need to be more code there to handle initialization correctly… but the basic idea would be that a macro here could synthesize a getter on my let variables (which the developer indicated are readonly) and a getter and a setter on my var variables (which the developer indicated are read write). The developer making use of this macro uses the standard swift conventions of expressing mutability.

If the AccessorMacro implementation was updated to only work on var variables… a macro like my example would still be possible as long as the struct contained only var variables.

@Storage struct Person {
  var id: String
  var name: String
}

Conceptually… it doesn't really make sense for this id to change over time. I'm asking the developer to shift their mental model for expressing mutability. I could introduce some kind of additional hint to the Storage macro here:

@Storage struct Person {
  @ReadOnly var id: String
  var name: String
}

Which then kind of scrambles up the public interface and requirements for a developer to make use of this hypothetical Storage macro.

I do not (yet) have a repo I can show to you implementing this pattern… but I did have something in mind that would have expected AccessorMacro to work on a let variable. My macro would have just only synthesized a getter on that let. I think I would be all in favor of an AccessorMacro that could (in theory) prevent a developer from synthesizing a setter on a let variable.

If you see the original test code I linked against it was the diff to correctly move around the case when a variable is initialized with an initial value when declared. So I actually thought there was a precedent to "transforming" the variable declaration (in this case removing the initial value). My first thought when filing this task was that applying an AccessorMacro would then just transform that let declaration to a var declaration.

[ahoppen]

My main concern here is that currently any let variable is guaranteed not to change by the type system. If we do officially allow macros to add getters to these let declarations, you need to hunt up the entire syntax stack to see if there is a macro that might be adding an accessor to the variable, which significantly hinders the ability to reason about source code locally.

[vanvoorden]

If we do officially allow macros to add getters to these let declarations, you need to hunt up the entire syntax stack to see if there is a macro that might be adding an accessor to the variable, which significantly hinders the ability to reason about source code locally.

@ahoppen Hmm… well… if the POV consensus is that accessor macros applied to let declarations is a bug to begin with then I suppose I don't have much to back up my belief it should stay in. Maybe I could make the argument that "changing a constant" with a macro is somehow picking up a "Smalltalk" style tradition of hacking your language… but that's also not the typical Swift philosophy.

Macros "are" Swift… but are also opportunities to step outside conventional Swift (that might not otherwise compile directly or easily). I guess I looked at Macros (and also Result Builders) as a place to bend the language around and where we give engineers flexibility (taking into account that flexibility needs to be managed responsibly and can harm as much as help). But I can see how there come times to move the debate back in favor of "legit" Swift code.

If we did plan to disallow this functionality… would you consider giving engineers a heads up (even before a diff is ready)? I feel like some engineers might have discovered this hack and might be trying to ship something that expects macros on let was legit and supported.

[ahoppen]

If we did plan to disallow this functionality… would you consider giving engineers a heads up (even before a diff is ready)? I feel like some engineers might have discovered this hack and might be trying to ship something that expects macros on let was legit and supported.

Disallowing getters on let might be guarded behind the Swift 6 language mode. But I won’t be alone in deciding that.