From c62b1bf2374c061e9254ba7a60c955075c7b93d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Rutkowski?= Date: Thu, 1 Oct 2020 18:35:20 +0200 Subject: [PATCH 1/8] Fix public extensions exposing nested code of all access levels --- Sources/SwiftDoc/Symbol.swift | 6 +- Tests/SwiftDocTests/InterfaceTypeTests.swift | 64 ++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/Sources/SwiftDoc/Symbol.swift b/Sources/SwiftDoc/Symbol.swift index 02514f8e..c8982bad 100644 --- a/Sources/SwiftDoc/Symbol.swift +++ b/Sources/SwiftDoc/Symbol.swift @@ -43,7 +43,11 @@ public final class Symbol { if let `extension` = `extension`, `extension`.modifiers.contains(where: { $0.name == "public" }) { - return true + + let isNotLimitedAccessModifier = { (modifier: Modifier) in + modifier.name != "internal" && modifier.name != "fileprivate" && modifier.name != "private" + } + return api.modifiers.allSatisfy(isNotLimitedAccessModifier) } if let symbol = context.compactMap({ $0 as? Symbol }).last, diff --git a/Tests/SwiftDocTests/InterfaceTypeTests.swift b/Tests/SwiftDocTests/InterfaceTypeTests.swift index 986e92e0..19212e40 100644 --- a/Tests/SwiftDocTests/InterfaceTypeTests.swift +++ b/Tests/SwiftDocTests/InterfaceTypeTests.swift @@ -69,4 +69,68 @@ final class InterfaceTypeTests: XCTestCase { XCTAssertEqual(symbol.api.name, "A") } } + + func testFunctionsInPublicExtension() throws { + let source = #""" + public extension Int { + func a() {} + public func b() {} + internal func c() {} + fileprivate func d() {} + private func e() {} + } + """# + + let url = try temporaryFile(contents: source) + let sourceFile = try SourceFile(file: url, relativeTo: url.deletingLastPathComponent()) + let module = Module(name: "Module", sourceFiles: [sourceFile]) + + XCTAssertEqual(sourceFile.symbols.count, 5) + XCTAssertTrue(sourceFile.symbols[0].isPublic, "Function `a()` should BE marked as public - its visibility is specified by extension") + XCTAssertTrue(sourceFile.symbols[1].isPublic, "Function `b()` should BE marked as public - its visibility is public") + XCTAssertFalse(sourceFile.symbols[2].isPublic, "Function `c()` should NOT be marked as public - its visibility is internal") + XCTAssertFalse(sourceFile.symbols[3].isPublic, "Function `d()` should NOT be marked as public - its visibility is fileprivate") + XCTAssertFalse(sourceFile.symbols[4].isPublic, "Function `e()` should NOT be marked as public - its visibility is private") + + XCTAssertEqual(module.interface.symbols.count, 2) + XCTAssertEqual(module.interface.symbols[0].name, "a()", "Function `a()` should be in documented interface") + XCTAssertEqual(module.interface.symbols[1].name, "b()", "Function `b()` should be in documented interface") + } + + func testNestedPropertiesInPublicExtension() throws { + let source = #""" + public class RootController {} + + public extension RootController { + class ControllerExtension { + public var public_properties: ExtendedProperties = ExtendedProperties() + internal var internal_properties: InternalProperties = InternalProperties() + } + } + + public extension RootController.ControllerExtension { + struct ExtendedProperties { + public var public_prop: Int = 1 + } + } + + internal extension RootController.ControllerExtension { + struct InternalProperties { + internal var internal_prop: String = "FOO" + } + } + """# + + + let url = try temporaryFile(contents: source) + let sourceFile = try SourceFile(file: url, relativeTo: url.deletingLastPathComponent()) + let module = Module(name: "Module", sourceFiles: [sourceFile]) + + XCTAssertEqual(module.interface.symbols.count, 5) + XCTAssertEqual(module.interface.symbols[0].name, "RootController") + XCTAssertEqual(module.interface.symbols[1].name, "ControllerExtension") + XCTAssertEqual(module.interface.symbols[2].name, "public_properties") + XCTAssertEqual(module.interface.symbols[3].name, "ExtendedProperties") + XCTAssertEqual(module.interface.symbols[4].name, "public_prop") + } } From 82bb27a99e58ff495376132cd382cab542992ed5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Rutkowski?= Date: Sun, 4 Oct 2020 16:57:55 +0200 Subject: [PATCH 2/8] Inline access checking closure Co-authored-by: Max Desiatov --- Sources/SwiftDoc/Symbol.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Sources/SwiftDoc/Symbol.swift b/Sources/SwiftDoc/Symbol.swift index c8982bad..68dc365f 100644 --- a/Sources/SwiftDoc/Symbol.swift +++ b/Sources/SwiftDoc/Symbol.swift @@ -44,10 +44,9 @@ public final class Symbol { if let `extension` = `extension`, `extension`.modifiers.contains(where: { $0.name == "public" }) { - let isNotLimitedAccessModifier = { (modifier: Modifier) in + return api.modifiers.allSatisfy { modifier in modifier.name != "internal" && modifier.name != "fileprivate" && modifier.name != "private" } - return api.modifiers.allSatisfy(isNotLimitedAccessModifier) } if let symbol = context.compactMap({ $0 as? Symbol }).last, From 5acfbcda4b8327dcf55dd174e57a8d407ff33752 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Sun, 4 Oct 2020 16:59:46 +0100 Subject: [PATCH 3/8] Update Changelog.md --- Changelog.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Changelog.md b/Changelog.md index 715bf2f3..94bf51c7 100644 --- a/Changelog.md +++ b/Changelog.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Fixed public extensions exposing nested code of all access levels. + #195 by Tunous. + ## [1.0.0-beta.5] - 2020-09-29 ### Added From 9a8ee7beace4173dca6b4282b403bb1a9c4fe5dc Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Sun, 4 Oct 2020 17:00:02 +0100 Subject: [PATCH 4/8] Update Changelog.md --- Changelog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index 94bf51c7..9a1eb25b 100644 --- a/Changelog.md +++ b/Changelog.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fixed public extensions exposing nested code of all access levels. - #195 by Tunous. + #195 by @Tunous. ## [1.0.0-beta.5] - 2020-09-29 From 06f41675004cd5ec81edec951a31e821404695a4 Mon Sep 17 00:00:00 2001 From: Mattt Date: Tue, 6 Oct 2020 10:33:09 -0700 Subject: [PATCH 5/8] Add testComputedPropertiesInPublicExtension --- Tests/SwiftDocTests/InterfaceTypeTests.swift | 27 ++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/Tests/SwiftDocTests/InterfaceTypeTests.swift b/Tests/SwiftDocTests/InterfaceTypeTests.swift index 19212e40..d5963073 100644 --- a/Tests/SwiftDocTests/InterfaceTypeTests.swift +++ b/Tests/SwiftDocTests/InterfaceTypeTests.swift @@ -97,6 +97,33 @@ final class InterfaceTypeTests: XCTestCase { XCTAssertEqual(module.interface.symbols[1].name, "b()", "Function `b()` should be in documented interface") } + func testComputedPropertiesInPublicExtension() throws { + let source = #""" + public extension Int { + var a: Int { 1 } + public var b: Int { 1 } + internal var c: Int { 1 } + fileprivate var d: Int { 1 } + private var e: Int { 1 } + } + """# + + let url = try temporaryFile(contents: source) + let sourceFile = try SourceFile(file: url, relativeTo: url.deletingLastPathComponent()) + let module = Module(name: "Module", sourceFiles: [sourceFile]) + + XCTAssertEqual(sourceFile.symbols.count, 5) + XCTAssertTrue(sourceFile.symbols[0].isPublic, "Property `a` should BE marked as public - its visibility is specified by extension") + XCTAssertTrue(sourceFile.symbols[1].isPublic, "Property `b` should BE marked as public - its visibility is public") + XCTAssertFalse(sourceFile.symbols[2].isPublic, "Property `c` should NOT be marked as public - its visibility is internal") + XCTAssertFalse(sourceFile.symbols[3].isPublic, "Property `d` should NOT be marked as public - its visibility is fileprivate") + XCTAssertFalse(sourceFile.symbols[4].isPublic, "Property `e` should NOT be marked as public - its visibility is private") + + XCTAssertEqual(module.interface.symbols.count, 2) + XCTAssertEqual(module.interface.symbols[0].name, "a", "Property `a` should be in documented interface") + XCTAssertEqual(module.interface.symbols[1].name, "b", "Property `b` should be in documented interface") + } + func testNestedPropertiesInPublicExtension() throws { let source = #""" public class RootController {} From b7ee28f2b0eb017ab3b7a59cafaad7740c259378 Mon Sep 17 00:00:00 2001 From: Mattt Date: Tue, 6 Oct 2020 10:33:23 -0700 Subject: [PATCH 6/8] Add testComputedPropertiesWithMultipleAccessModifiersInPublicExtension --- Tests/SwiftDocTests/InterfaceTypeTests.swift | 39 ++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/Tests/SwiftDocTests/InterfaceTypeTests.swift b/Tests/SwiftDocTests/InterfaceTypeTests.swift index d5963073..d5ec896b 100644 --- a/Tests/SwiftDocTests/InterfaceTypeTests.swift +++ b/Tests/SwiftDocTests/InterfaceTypeTests.swift @@ -124,6 +124,45 @@ final class InterfaceTypeTests: XCTestCase { XCTAssertEqual(module.interface.symbols[1].name, "b", "Property `b` should be in documented interface") } + func testComputedPropertiesWithMultipleAccessModifiersInPublicExtension() throws { + let source = #""" + public extension Int { + internal(set) var a: Int { + get { 1 } + set {} + } + private(set) var b: Int { + get { 1 } + set {} + } + public internal(set) var c: Int { + get { 1 } + set {} + } + public private(set) var d: Int { + get { 1 } + set {} + } + } + """# + + let url = try temporaryFile(contents: source) + let sourceFile = try SourceFile(file: url, relativeTo: url.deletingLastPathComponent()) + let module = Module(name: "Module", sourceFiles: [sourceFile]) + + XCTAssertEqual(sourceFile.symbols.count, 4) + XCTAssertTrue(sourceFile.symbols[0].isPublic, "Property `a` should be marked as public - the visibility of its getter is public") + XCTAssertTrue(sourceFile.symbols[0].isPublic, "Property `b` should be marked as public - the visibility of its getter is public") + XCTAssertTrue(sourceFile.symbols[0].isPublic, "Property `c` should be marked as public - the visibility of its getter is public") + XCTAssertTrue(sourceFile.symbols[0].isPublic, "Property `d` should be marked as public - the visibility of its getter is public") + + XCTAssertEqual(module.interface.symbols.count, 4) + XCTAssertEqual(module.interface.symbols[0].name, "a", "Property `a` should be in documented interface") + XCTAssertEqual(module.interface.symbols[1].name, "b", "Property `b` should be in documented interface") + XCTAssertEqual(module.interface.symbols[2].name, "c", "Property `c` should be in documented interface") + XCTAssertEqual(module.interface.symbols[3].name, "d", "Property `d` should be in documented interface") + } + func testNestedPropertiesInPublicExtension() throws { let source = #""" public class RootController {} From 983670e9aaf0740c5d4c1750b2e671781ff02a93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Rutkowski?= Date: Wed, 7 Oct 2020 18:13:56 +0200 Subject: [PATCH 7/8] Do not skip properties which have limited access of setters only --- Sources/SwiftDoc/Symbol.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SwiftDoc/Symbol.swift b/Sources/SwiftDoc/Symbol.swift index 68dc365f..1b376a11 100644 --- a/Sources/SwiftDoc/Symbol.swift +++ b/Sources/SwiftDoc/Symbol.swift @@ -45,7 +45,7 @@ public final class Symbol { `extension`.modifiers.contains(where: { $0.name == "public" }) { return api.modifiers.allSatisfy { modifier in - modifier.name != "internal" && modifier.name != "fileprivate" && modifier.name != "private" + modifier.detail != nil || (modifier.name != "internal" && modifier.name != "fileprivate" && modifier.name != "private") } } From 3b35ba0337a4f35b9661c4c2884f3d17fc94fc7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Rutkowski?= Date: Wed, 7 Oct 2020 18:19:22 +0200 Subject: [PATCH 8/8] Add missing case and fix incorrect checks in access level tests --- Tests/SwiftDocTests/InterfaceTypeTests.swift | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/Tests/SwiftDocTests/InterfaceTypeTests.swift b/Tests/SwiftDocTests/InterfaceTypeTests.swift index d5ec896b..0c3db153 100644 --- a/Tests/SwiftDocTests/InterfaceTypeTests.swift +++ b/Tests/SwiftDocTests/InterfaceTypeTests.swift @@ -139,7 +139,11 @@ final class InterfaceTypeTests: XCTestCase { get { 1 } set {} } - public private(set) var d: Int { + public fileprivate(set) var d: Int { + get { 1 } + set {} + } + public private(set) var e: Int { get { 1 } set {} } @@ -150,17 +154,19 @@ final class InterfaceTypeTests: XCTestCase { let sourceFile = try SourceFile(file: url, relativeTo: url.deletingLastPathComponent()) let module = Module(name: "Module", sourceFiles: [sourceFile]) - XCTAssertEqual(sourceFile.symbols.count, 4) + XCTAssertEqual(sourceFile.symbols.count, 5) XCTAssertTrue(sourceFile.symbols[0].isPublic, "Property `a` should be marked as public - the visibility of its getter is public") - XCTAssertTrue(sourceFile.symbols[0].isPublic, "Property `b` should be marked as public - the visibility of its getter is public") - XCTAssertTrue(sourceFile.symbols[0].isPublic, "Property `c` should be marked as public - the visibility of its getter is public") - XCTAssertTrue(sourceFile.symbols[0].isPublic, "Property `d` should be marked as public - the visibility of its getter is public") + XCTAssertTrue(sourceFile.symbols[1].isPublic, "Property `b` should be marked as public - the visibility of its getter is public") + XCTAssertTrue(sourceFile.symbols[2].isPublic, "Property `c` should be marked as public - the visibility of its getter is public") + XCTAssertTrue(sourceFile.symbols[3].isPublic, "Property `d` should be marked as public - the visibility of its getter is public") + XCTAssertTrue(sourceFile.symbols[4].isPublic, "Property `e` should be marked as public - the visibility of its getter is public") - XCTAssertEqual(module.interface.symbols.count, 4) + XCTAssertEqual(module.interface.symbols.count, 5) XCTAssertEqual(module.interface.symbols[0].name, "a", "Property `a` should be in documented interface") XCTAssertEqual(module.interface.symbols[1].name, "b", "Property `b` should be in documented interface") XCTAssertEqual(module.interface.symbols[2].name, "c", "Property `c` should be in documented interface") XCTAssertEqual(module.interface.symbols[3].name, "d", "Property `d` should be in documented interface") + XCTAssertEqual(module.interface.symbols[4].name, "e", "Property `e` should be in documented interface") } func testNestedPropertiesInPublicExtension() throws {