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

Fixes all the compiler warnings #82

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

xanathar
Copy link
Contributor

This fixes all compiler warnings and all clippy warnings. I highlight some weird cases in the diff below, for you to double-check.

let mut unwrapped_items: Vec<&syn::Item> = vec![];
for item in ast {
match item {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This match was weird.
Is it correct that in case it matches an Item::Mod(...) but without a content that it does nothing?
Current implementation reflects original code, not sure if it reflects the intention though.

@@ -463,20 +459,16 @@ pub fn build_method_delegate_if_required(
method_name: &String,
parameter_name: &String,
) -> Option<String> {
let emit_from_struct = false;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is weird. emit_from_struct is always false (unless I'm wrong and missing something big) which could be a choice for readability. But it's not a const, and has implications in the next line.

match &me.type_kind {
TypeKind::Function(parameters, return_type) => {
if emit_from_struct && !options.csharp_use_function_pointer {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So... if emit_from_struct is always false, can this actually ever be the case? I removed the branch; this was likely a copy/paste leftover, but might be worth checking if it isn't some kind of bug.

@neuecc
Copy link
Member

neuecc commented Jul 8, 2024

Ah, I see, the point you raised seems to be an actual mistake.
Can you please correct the conflict?

@xanathar xanathar force-pushed the pr/make-clippy-happy branch from a0e36cc to 54618c5 Compare July 9, 2024 23:14
@xanathar
Copy link
Contributor Author

xanathar commented Jul 9, 2024

I also rebased this and ran cargo fmt.

I will be away from any sort of keyboards until the end of July (vacation time!). If you find anything wrong, I can check the PRs as soon as I'm back.

@xanathar xanathar force-pushed the pr/make-clippy-happy branch from 54618c5 to afbab0f Compare August 1, 2024 16:08
@xanathar
Copy link
Contributor Author

xanathar commented Aug 1, 2024

Rebased and fmt'ed again.

Copy link
Contributor

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 30 days.

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.

2 participants