Replace private modifier node with field on supporting types
#11346
Conversation
kazcw
added
the
CI: No changelog needed
kazcw
requested review from
4e6,
JaroslavTulach,
hubertp,
Akirathan,
Frizi,
farmaazon,
vitvakatu,
AdRiley,
PabloBuchu,
indiv0,
somebody1234 and
MrFlashAccount
as code owners
… types. Also: - Rust parser tests: Switch to a builder-style API for defining expected `Function` ASTs to allow further changes to `Function` fields without rewriting all the tests again. - TreeToIr: Fix discarded module-level `diag`; add a test that covers module diagnostics. - Syntax: Disallow `private` methods in function blocks.
kazcw
force-pushed
the
wip/kw/private-field
branch
from
3c6a075 to
bfd074f
Compare
| private | ||
| private | ||
| """); | ||
| assertSingleSyntaxError( |
There was a problem hiding this comment.
There was a problem hiding this comment.
There's no risk of rejecting this due to the one-private-declaration-per-module rule--the second private is not a declaration but a field of the Function.
| Some(statement) => Tree::app( | ||
| private.with_error(match visibility_context { | ||
| VisibilityContext::Public => SyntaxError::StmtUnexpectedPrivateSubject, | ||
| VisibilityContext::Private => SyntaxError::StmtUnexpectedPrivateContext, | ||
| }), | ||
| statement, | ||
| ), |
There was a problem hiding this comment.
It took me a good while to understand, why do we always set error here. I assumed that the responsibility of this function is to make statement a private object, not that it is already applied and any excessive private is an error. Please rename/add short docs for this function.
| fn into_private_keyword(item: Item) -> token::PrivateKeyword { | ||
| let Item::Token(keyword) = item else { unreachable!() }; | ||
| let token::Variant::PrivateKeyword(variant) = keyword.variant else { unreachable!() }; | ||
| keyword.with_variant(variant) |
There was a problem hiding this comment.
The same function is in statement.rs
There was a problem hiding this comment.
Implemented some downcasting helpers so we don't need a special function for this case.
| for token in keywords { | ||
| let Item::Token(keyword) = token else { unreachable!() }; | ||
| let token::Variant::PrivateKeyword(variant) = keyword.variant else { unreachable!() }; | ||
| let keyword = keyword.with_variant(variant); |
There was a problem hiding this comment.
Another into_private_keyword logic
lib/rust/parser/src/syntax/tree.rs
Outdated
| /// A `private` declaration. | ||
| Private { | ||
| pub keyword: token::Private<'s>, | ||
| pub body: Option<Tree<'s>>, | ||
| pub keyword: token::PrivateKeyword<'s>, |
There was a problem hiding this comment.
This is for making a module private, right? I would then add this info here.
Pull Request Description
Eliminate
privatemodifier-node:privateis a field in supporting types, or a single-token node in the case ofprivatedeclarations.Important Notes
FunctionASTs to allow further changes toFunctionfields without rewriting all the tests again.diag; add a test that covers module diagnostics.privatemethods in function blocks. (Previously this was enforced in the compiler.)Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
or the Snowflake database integration, a run of the Extra Tests has been scheduled.