Skip to content

Change error diagnosed for when self isn't available in scope #59140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jun 3, 2022
Merged

Change error diagnosed for when self isn't available in scope #59140

merged 10 commits into from
Jun 3, 2022

Conversation

NSAntoine
Copy link
Contributor

As discussed on https://forums.swift.org/t/changing-error-diagnosed-for-when-self-isn-t-available/57616, the purpose of these changes are to hopefully make it more obvious, especially to beginner programmers, what the purpose of self is, as these changes will diagnose cannot find self in scope, did you mean to declare or extend a type rather than just cannot find self in scope when self isn't available.

@NSAntoine NSAntoine marked this pull request as draft May 28, 2022 10:12
@NSAntoine
Copy link
Contributor Author

@AnthonyLatsis test?

@hamishknight
Copy link
Contributor

@swift-ci please test

@NSAntoine
Copy link
Contributor Author

@hamishknight Fixed compilation errors, try to test it again, also, every test file should have // RUN: %target-typecheck-verify-swift at the top, right?

Copy link
Contributor

@hamishknight hamishknight left a comment

Choose a reason for hiding this comment

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

Nice!

@hamishknight
Copy link
Contributor

Every test file should have // RUN: %target-typecheck-verify-swift at the top, right?

Yup, every test that needs to run a typecheck over the test file

@hamishknight
Copy link
Contributor

@swift-ci please test

@NSAntoine
Copy link
Contributor Author

@AnthonyLatsis addressed all concerns, could you try test now?

@AnthonyLatsis
Copy link
Collaborator

@swift-ci please smoke test

@NSAntoine
Copy link
Contributor Author

@AnthonyLatsis could you ask it to build a toolchain aswell? The tests pass and everything works but I’m just curious to try out locally

@AnthonyLatsis
Copy link
Collaborator

@swift-ci please build toolchain macOS

@NSAntoine NSAntoine marked this pull request as ready for review May 28, 2022 17:59
@NSAntoine
Copy link
Contributor Author

@xwu Ready for merge now?

@AnthonyLatsis AnthonyLatsis requested review from xedin and hborla May 28, 2022 22:01
@AnthonyLatsis
Copy link
Collaborator

Toolchain: https://ci.swift.org/view/all/job/swift-PR-toolchain-macos/179/

Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@xedin
Copy link
Contributor

xedin commented Jun 2, 2022

@SerenaKit You might want to consider a tailored diagnostic like this for Self as well.

@xedin
Copy link
Contributor

xedin commented Jun 2, 2022

@swift-ci please test

@AnthonyLatsis
Copy link
Collaborator

@SerenaKit I'm going to squash this so that we don't pollute the commit history with review-specific amendments.

@AnthonyLatsis AnthonyLatsis merged commit 99116c9 into swiftlang:main Jun 3, 2022
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.

5 participants