Skip to content

Commit

Permalink
[BUGFIX] Fixing hasSetter bug on variable (#37)
Browse files Browse the repository at this point in the history
Fixed bug where variable would return true when only contain a get accessor, added support for willSet/didSet
  • Loading branch information
mobrien-ghost authored Mar 18, 2024
1 parent a55babf commit 3f6de1c
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,19 @@ struct VariableSemanticsResolver: SemanticsResolving {

func resolveHasSetter() -> Bool {
// If setter exists in accessors can return true
if resolveAccessors().contains(where: { $0.kind == .set }) {
let hasSetterAccessor = resolveAccessors().contains(where: { $0.kind == .set || $0.kind == .willSet })
if hasSetterAccessor {
return true
}
// Otherwise if the keyword is not `let` (immutable)
guard resolveKeyword() != "let" else { return false }
// Check if modifiers contain a private set
guard !resolveModifiers().contains(where: { $0.name == "private" && $0.detail == "set" }) else { return false }
// Finally if the root context is not a protocol, and the keyword is var, it can have a setter
return node.context?.as(ProtocolDeclSyntax.self) == nil && resolveKeyword() == "var"
let isPotential = node.context?.as(ProtocolDeclSyntax.self) == nil && resolveKeyword() == "var"
if resolveAccessors().isEmpty {
return isPotential
}
return isPotential && hasSetterAccessor
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ public struct Accessor: DeclarationComponent, SyntaxChildCollecting {

/// A setter that sets a value.
case set

/// Accessor invoked when something has been set.
case didSet

/// Accessor invoked when something is about to be set.
case willSet
}

// MARK: - Properties: DeclarationComponent
Expand Down
4 changes: 4 additions & 0 deletions Tests/SyntaxSparrowTests/Declarations/VariableTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,7 @@ final class VariableTests: XCTestCase {

let variable = instanceUnderTest.variables[0]
XCTAssertEqual(variable.accessors.count, 2)
XCTAssertTrue(variable.hasSetter)
// Get
XCTAssertEqual(variable.accessors[0].kind, .get)
let getterBody = try XCTUnwrap(variable.accessors[0].body)
Expand Down Expand Up @@ -465,6 +466,7 @@ final class VariableTests: XCTestCase {

let variable = instanceUnderTest.variables[0]
XCTAssertEqual(variable.accessors.count, 1)
XCTAssertFalse(variable.hasSetter)
// Get
XCTAssertEqual(variable.accessors[0].kind, .get)
XCTAssertEqual(variable.accessors[0].effectSpecifiers?.throwsSpecifier, "throws")
Expand All @@ -488,6 +490,7 @@ final class VariableTests: XCTestCase {

let variable = instanceUnderTest.variables[0]
XCTAssertEqual(variable.accessors.count, 1)
XCTAssertFalse(variable.hasSetter)
// Get
XCTAssertEqual(variable.accessors[0].kind, .get)
XCTAssertNil(variable.accessors[0].effectSpecifiers?.throwsSpecifier)
Expand All @@ -511,6 +514,7 @@ final class VariableTests: XCTestCase {

let variable = instanceUnderTest.variables[0]
XCTAssertEqual(variable.accessors.count, 1)
XCTAssertFalse(variable.hasSetter)
// Get
XCTAssertEqual(variable.accessors[0].kind, .get)
XCTAssertEqual(variable.accessors[0].effectSpecifiers?.throwsSpecifier, "throws")
Expand Down

0 comments on commit 3f6de1c

Please sign in to comment.