API: Use a ModItem as the root of a crate
#306
Conversation
xFrednet
added
C-enhancement
| span: SpanId(..), | ||
| vis: Visibility {{ /* WIP: See rust-marker/marker#26 */}}, | ||
| ident: Ident { | ||
| name: "", |
There was a problem hiding this comment.
This ident is suspiciously empty. Shouldn't it be None?
There was a problem hiding this comment.
I'm not quite sure how this value was constructed. I've now removed all compiler generated items from Marker's AST representation and this "fixed" (meaning hid this issue) for now. I believe I have seen some circumstances, where the identifier was empty of something in the compiler for a valid reason, but I don't remember where. As for the None part. The Ident struct is used a lot, adding an option in it, would make handling it very cumber sum. So, even if this was a bug, I think it's better to have "" as a symbol than using an option inside the Ident
| span: $DIR/print_me_root_module.rs:1:1 - 1:1, | ||
| }, | ||
| }, | ||
| use_path: AstPath { |
There was a problem hiding this comment.
I'd be glad if lints didn't even need to see the standard prelude. I guess it deserves it's own ugly crutch to skip such code. And.. in general, I think lints don't ever need to see code with the "built-in" spans auto-generated by the compiler. I imagine someone writes a lint that triggers on a standard prelude use item and thus is cursed to never be able to ignore that false positive unless ignoring it at crate-level meaning the lint is just disabled and useless 😺
There was a problem hiding this comment.
Skipping the import items from the prelude would be pretty simple, items already have an ugly crunch to handle such things.
The question for me, is more if we want this? So these items are definitely not intended for lint emissions, but they are required to get the full information about what namespace etc are available. On the other hand, these changes are always bound to Editions, so if we provide information about that, it should be sufficient to get all the information needed for linting 🤔
There was a problem hiding this comment.
The namespace information should be queried from the TyCtxt. I don't think that lint authors will specifically traverse all uses and do manual name resolution, which is a complex and recursive algorithm.
marker_api/src/span.rs
Outdated
| @@ -235,7 +235,7 @@ impl<'ast> std::fmt::Debug for Span<'ast> { | |||
| fmt_pos(file.try_to_file_pos(self.end)) | |||
| ), | |||
| SpanSource::Macro(expn) => format!("[Inside Macro] {:#?}", expn.call_site()), | |||
| SpanSource::Buildin(_) => "[From Prelude]".to_string(), | |||
| SpanSource::Builtin(_) => "[From Prelude]".to_string(), | |||
There was a problem hiding this comment.
If lints can't observe the prelude, then we may go without the "Builtin" span source? Or is there any other way this "Builtin" span source can be created? The test that was added in this PR no longer contains such a span source. If there is any other way it could be created, it would be good to have a test that showcases how it can be constructed
There was a problem hiding this comment.
The filter mechanism added in this PR should filter out all spans with a builtin source. However, I'm not 100% sure if that really removes every trace. Before this new SpanSource Marker would ICE when requesting these spans. I'm in favor of keeping it in the API as it will prevent potential ICEs. If we're starting to look at the stabilization of Marker, we can review if this value ever came up again.
I can imagine that also some unstable features might fall back on this source, during initial development. This is just a guess, though.
I'll change the text of this function to say "[Builtin]" instead.
This PR has 3 major changes:
Crateare now wrapped in aModItem, that is the root module of the crate.LintPasstrait now as a newcheck_cratemethod.It has been some time, but I'm back. Exams really took it out of me, and then I felt so exhausted, that I needed to take a step back for some time, just to breathe. This should hopefully be the kickoff for me getting back into things :D
Closes #279
Feedback is as always highly appreciated! And for everyone reading this: I hope you have a beautiful day. It's awesome that you're a part of open source, be proud of yourself! Life can be stressful, so don't forget to take a break every once in a while.