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

Separate analyze with dmd and autofix flows from libdparse analyze flow #142

Merged
merged 4 commits into from
Sep 25, 2024

Conversation

Vladiwostok
Copy link
Collaborator

Break down different flows in separate files so that it's easier to work on individual flows

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is no longer used at all in the project

import dscanner.analysis.unused_result : UnusedResultChecker;
import dscanner.analysis.unused_variable : UnusedVariableCheck;
import dscanner.analysis.useless_assert : UselessAssertCheck;
import dscanner.analysis.useless_initializer : UselessInitializerChecker;
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not simply import dscanner.analysis?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's much more cleaner this way. The idea is to organize in the future the src files in analysis dir in other dirs, it will be easier then

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see how importing dscanner.analysis is less cleaner even if you do shuffle files inside the analysis directory.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's easier to see which files and which declarations are actually used when refactoring. A full dscanner.analysis import would also include other modules unrelated to the checks.

Without specific imports, you either have to read the code to verify used imports on rely on compiler errors.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the main file that uses all the checks. Most of the files in the analysis folder will be needed and all of them implement a single visitor (so you could even make them general imports, not selective ones), so I don't see the argument for spelling them out (and with selective imports too). This to me seems java-level prolixity, however, this really isn't a debate worth our time - do as you wish.

src/dscanner/analysis/run.d Outdated Show resolved Hide resolved
src/dscanner/analysis/autofix.d Outdated Show resolved Hide resolved
@RazvanN7 RazvanN7 merged commit cd503ef into Dlang-UPB:replace_libdparse Sep 25, 2024
17 checks passed
@Vladiwostok Vladiwostok deleted the autofix-flow branch September 25, 2024 10:16
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