API: Big API Module Migration

[xFrednet]

Summary

• Moved semantic types and generics to the new marker_api::sem module
• Moved common items, like IDs, to the new marker_api::common module
• Removed the Sem and Syn prefix from types and generics
• marker_api::prelude no longer contains the semantic and syntactic TyKind enums
• marker_api::prelude now imports the sem and ast names
• The marker_api::ast module has been flattened
• The marker_api::lint and marker_api::interface are now private

Motivation

Moving modules

The ast module of marker_api contained several items, which were not part of the syntactic representation. The semantic type representation is an example of something which didn't quite belong in the AST module.

Rustc also has this separation with rustc_middle providing the semantic type representation and rustc_hir holding the HIR representation, with an AST structure and spans etc. (It's more complicated than this, but you get the idea)

This is one of the biggest breaking changes I've done in Marker so far. It's better to make this separation now, while there are very very few to no users.

Removing the Sem and Syn prefix

This is a change I'm actually uncertain about. The Sem and Syn prefixes were previously used to distinguish between the semantic and syntactic variants of types. For example, SynTyKind and SemTyKind. Now that they live in different modules, they can have the same name.

I believe that common coding conventions dictate, that the Syn and Sem prefix should be removed, as it's just a repetition of the module, and they make the name more verbose than it needs to be.

The reason, why I'm unsure about this change, is that I liked the name-based distinction. When reviewing code and reading SemTyKind it was 100% clear which kind of type this is. Now a user will probably import the TyKind. When reading something like this TyKind::Ref(x) = expr.ty(), it requires more context to know if this is a semantic or syntactic type. Users can still specify the path, like this sem::ty::TyKind but this is more verbose, than the previous SemTyKind.

Another advantage was, that the documentation only had one entry for every type. Searching for SemRefTy provided one result, while RefTy will from now on provide two, one from the semantics and one from the AST module.

It would be good to know, how often code mixes the two different TyKinds. That might give a good indicator if the types should have the prefixes or not. In Clippy, we rarely mix the types in one file. There are not that many lints, which require information from both kinds. Semantic types are often not even stored, but only passed to the next function to check if they belong to a specific type or implement a trait.

---

@Veetaha If you have the time, would you mind reading the PR description, and provide feedback if this sounds reasonable?

I doubt that reviewing the actual code changes is worth it, I only moved and renamed a lot of things. The only thing I added was a short doc comment in the sem and ast module.

This should be the last PR fore v0.3.0. I didn't think we would make it this far in so little time. We can be proud of the progress 😊

[Veetaha]

The changes sound reasonable. I would go further and maybe think of making the public module structure more flat. For example reduce the sem::ty::TyKind to just sem::TyKind. This should make it easier to use the types without importing them and still leaving the sem/ast prefix for clarity.

There could be an explicit recommendation about not importing ast/sem types and referencing them with ast::/sem:: prefix in the module doc comments. I think it's nice to have a three-symbol module name available via prelude that you could use as a dispatcher into the completions ast::<complete>, and leave it in code, because it doesn't take much space. For example, when writing proc macros I never import symbols from syn, I always spell the full path e.g. syn::LitStr.

Modules make it harder to use outside, and reexports should be used for that, unless there would be name conflicts and/or the module name bears an important part of the context for the type name. Both are the case for ast/sem, but not for ty/item/expr/... modules.

We could still have ty/item/expr... modules in code for internal organization, but make them private and instead reexport everything:

mod ty;
mod item;
mod export;
// ...

pub use ty::*;
pub use item::*;
pub use expr::*;
// ...

Same thing about common module. I see it is already reexporting its contents in root of marker_api 👍
The less public modules there are the less API surface there is and the more freedom there is in changing the module structure.

Btw. Syn vs Sem look just too similar to me because n is quite close to m, and mostly the y/e differentiates them, so I didn't like the existing prefixes because of that. ast/sem on the other hand are very distinct, and I like this more.

---

So in general LGTM

[Veetaha]

This doesn't look right as it complains about the module being private

[xFrednet]

The new ui_test version blesses tests by default. That's why I didn't see the fail. I don't like that, I'll make it error by default. There is a setting for that. (Will do that later)

Thank you!

[xFrednet]

Having everything in the root of ast and sem was something I considered. I like the simplicity, of just writing ast::XYZ or sem::XYZ without the need to always import every little type. That's also how I like to use the HIR module from rustc.

The main drawback I see, is the missing structure in the documentation. After this change, it's harder to find something in the marker_api::ast module due to its size.

I'm probably also a bit biased towards the pub module structure because that's just how the API was until now. I feel like the advantages and disadvantages are almost even. The latest commits now have the "flatter" module structure. I'm leaning towards this one, since you recommended it. But I'll still think a bit more about it, before making a final decision (tomorrow).

---

@xFrednet FIXME: Cleanup the namespace mess in the driver (I just fixed all the imports. Now, the used names in the driver are a mess. Sometimes they use the module ast:: and sometimes just import everything at the top)

[xFrednet]

I've rebased, to get the ui_test update in and also updated the changelog.

• Moved semantic types and generics to the new marker_api::sem module
• Moved common items, like IDs, to the new marker_api::common module
• Removed the Sem and Syn prefix from types and generics
• marker_api::prelude no longer contains the semantic and syntactic TyKind enums
• marker_api::prelude now imports the sem and ast names
• The marker_api::ast module has been flattened
• The marker_api::lint and marker_api::interface are now private