Skip to content

Conversation

skorulis-ap
Copy link
Collaborator

@skorulis-ap skorulis-ap commented Aug 13, 2025

The way I implemented this the first time helped to cleanup cash-ios but it didn't do a good job of surfacing the errors in a friendly way. This version raises errors in the same pathway for the nonAbstract error.

I realised that there are significant performance benefits possible by enforcing splitting out abstract assemblies so I got rid of it as a configurable parameter to allow deleting the replacement code in a future PR.

@skorulis-ap skorulis-ap changed the title Force strict abstract mode for assemblies Force strict abstract assemblies Aug 13, 2025
@skorulis-ap skorulis-ap marked this pull request as ready for review August 13, 2025 06:48
@skorulis-ap skorulis-ap requested a review from bradfol August 13, 2025 06:58
@skorulis-ap skorulis-ap force-pushed the skorulis/strict-abstract-error branch from f2c1121 to 1e723fa Compare August 13, 2025 22:18
Copy link
Collaborator

@bradfol bradfol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

Comment on lines -68 to -69
@Flag(help: "When true, only abstract modules will be allowed to contain abstract registrations")
var strictAbstract: Bool = false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

Comment on lines +21 to +26
fileprivate var supportedFunctions: [Registration.FunctionName] {
switch self {
case .abstractAssembly:
return [.registerAbstract]
case .moduleAssembly, .autoInitAssembly, .fakeAssembly:
return [.implements, .register]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice pattern

@skorulis-ap skorulis-ap merged commit 5e570aa into main Aug 13, 2025
3 checks passed
@skorulis-ap skorulis-ap deleted the skorulis/strict-abstract-error branch August 13, 2025 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants