Skip to content

Adding a global switch that will check for global header imports#28

Merged
dblock merged 8 commits intodblock:masterfrom
jeffctown:feature/AddingSupportForGlobalOrBracketNotationImports
Dec 15, 2018
Merged

Adding a global switch that will check for global header imports#28
dblock merged 8 commits intodblock:masterfrom
jeffctown:feature/AddingSupportForGlobalOrBracketNotationImports

Conversation

@jeffctown
Copy link
Copy Markdown
Collaborator

@jeffctown jeffctown commented Dec 13, 2018

I work daily on an iOS App that has multiple frameworks that live in the same repository and are used by my app. When I use fui on my repo, I end up getting a lot of false positives that tell me that my frameworks have unused imports when they are actually used. The reason the references are not found is because these framework classes are imported as global imports (#import <Framework/Class.h>).

This PR is to add a -g or --global switch to perform a regex against imports in this format. Adding a regex does decrease performance time, so I thought it made sense to have this switch default to false to not affect computation time for users who do not need this functionality.

Here are computation times for my project (it's a pretty big project, 500k+ lines).

Global Switch Disabled
real 3m38.529s
user 2m17.454s
sys 1m20.829s

Global Switch Enabled
real 5m20.673s
user 3m53.501s
sys 1m26.923s

Unit tests are included.

FYI - I've never contributed to a ruby project before, so please don't hesitate to tell me if there are obvious improvements that can be made to this pull request (thank you Rubocop!).

Copy link
Copy Markdown
Owner

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Looks good with a few changes, thinking out loud ...

I think if we are going to have switches, we should have both --global-imports and --local-imports, and I would not worry about performance and default both to true. To be consistent since we already have a x option that ignores something maybe we should have --ignoreglobal and --ignorelocal or if we want to make it prettier, --ignore-xib-files, --ignore-global-imports, --ignore-local-imports to skip? I think I would prefer a backwards incompatible change and do the latter because it's prettier.

The README needs an update too.

Comment thread lib/fui/finder.rb Outdated
@jeffctown
Copy link
Copy Markdown
Collaborator Author

jeffctown commented Dec 14, 2018

@dblock - I agree with the "prettier" switches that you described above, so I updated them to be --ignore-xib-files, --ignore-global-imports, and --ignore-local-imports. I also turned on the global checks by default.

@jeffctown
Copy link
Copy Markdown
Collaborator Author

README updated.

@dblock dblock merged commit 6bd65ce into dblock:master Dec 15, 2018
@dblock
Copy link
Copy Markdown
Owner

dblock commented Dec 15, 2018

Merged. Thank you.

Want to help me comaintain this thing @jeffctown? Maybe make the next release? Drop me an email to dblock at dblock dot org if you're interested with your rubygems username.

@jeffctown jeffctown deleted the feature/AddingSupportForGlobalOrBracketNotationImports branch December 15, 2018 21:00
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