Skip to content
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

proposal: change duchess::java_package! default visibility to pub(crate) #133

Open
rcoh opened this issue Apr 2, 2024 · 3 comments
Open

Comments

@rcoh
Copy link
Contributor

rcoh commented Apr 2, 2024

Currently the modules and types generated by java_package are all pub. This seems unlikely to be the right thing for most customers (for one thing, the documentation of java_package is hard to read and lacks variable names).

It seems like pub(crate) is probably the right option for modules. The inner types could be either pub or pub(crate)

We would then add another invocation path to access the old behavior of creating pub modules/structs. A few possible APIs for this:

// assuming we want to maintain the current invariant that the main block of the proc macro is _exactly_ the output from javap.
duchess::java_package!(vis: pub) {
    package auth;

    class Authenticated { * }
}

if we are OK breaking from the strict mapping between javap output:

duchess::java_package! {
    #[pub]
    package auth;

    #[pub]
    class Authenticated { * }
}

or addition of a new top level API:

duchess::public_java_package! {
    package auth;

    class Authenticated { * }
}

The last and final option is to provide no api for this and document workaround via pub use

@nikomatsakis
Copy link
Member

if we are OK breaking from the strict mapping between javap output:

I think it is important that it is possible to copy and paste javap into java_package, but I think it's also ok to have extra stuff (and indeed we already do, e.g., { * } not javap output).

@nikomatsakis
Copy link
Member

That said: It seems like the classes could basically always be public, it's really the packages that could default to pub(crate), no?

Then we could have pub package foo.bar if you really want to export it.

The only use case that doesn't work there is if you want some of the Java classes to be public, which seems pretty obscure to me.

@nikomatsakis
Copy link
Member

I imagine something like:

A given module is created as pub if there is a pub package that requires it to be.

Otherwise pub(crate).

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

No branches or pull requests

2 participants