search

unused-code / unused

A tool to identify potentially unused code.
https://unused.codes
MIT License
283 stars 11 forks source link

unused{-rs} could not parse the tags in an umbrella project (Elixir) #3

Closed milmazz closed 4 years ago

milmazz commented 4 years ago

Using the Default config setting, unused-rs (and also unused) failed to find the ctags file in an umbrella project, specifically if you run the command under a specific app.

How to reproduce

$ mix new demo --umbrella
```console * creating README.md * creating .formatter.exs * creating .gitignore * creating mix.exs * creating apps * creating config * creating config/config.exs Your umbrella project was created successfully. Inside your project, you will find an apps/ directory where you can create and host many apps: cd demo cd apps mix new my_app Commands like "mix compile" and "mix test" when executed in the umbrella project root will automatically run for each application in the apps/ directory. ``` 
$ cd demo/apps/
$ mix new app1
```console * creating README.md * creating .formatter.exs * creating .gitignore * creating mix.exs * creating lib * creating lib/app1.ex * creating test * creating test/test_helper.exs * creating test/app1_test.exs Your Mix project was created successfully. You can use "mix" to compile it, test it, and more: cd app1 mix test Run "mix help" for more commands. ``` 
$ mix new app2
```console * creating README.md * creating .formatter.exs * creating .gitignore * creating mix.exs * creating lib * creating lib/app2.ex * creating test * creating test/test_helper.exs * creating test/app2_test.exs Your Mix project was created successfully. You can use "mix" to compile it, test it, and more: cd app2 mix test Run "mix help" for more commands. ``` 
$ cd ../
$ mix test
```console ==> app1 Compiling 1 file (.ex) Generated app1 app ==> app2 Compiling 1 file (.ex) Generated app2 app ==> app1 .. Finished in 0.03 seconds 1 doctest, 1 test, 0 failures Randomized with seed 559487 ==> app2 .. Finished in 0.01 seconds 1 doctest, 1 test, 0 failures Randomized with seed 559487 ``` 
$ git init 
Initialized empty Git repository in /tmp/demo/.git/
$ git add .
$ git commit -m "initial structure"
```console [master (root-commit) 91a537d] initial structure 19 files changed, 287 insertions(+) create mode 100644 .formatter.exs create mode 100644 .gitignore create mode 100644 README.md create mode 100644 apps/app1/.formatter.exs create mode 100644 apps/app1/.gitignore create mode 100644 apps/app1/README.md create mode 100644 apps/app1/lib/app1.ex create mode 100644 apps/app1/mix.exs create mode 100644 apps/app1/test/app1_test.exs create mode 100644 apps/app1/test/test_helper.exs create mode 100644 apps/app2/.formatter.exs create mode 100644 apps/app2/.gitignore create mode 100644 apps/app2/README.md create mode 100644 apps/app2/lib/app2.ex create mode 100644 apps/app2/mix.exs create mode 100644 apps/app2/test/app2_test.exs create mode 100644 apps/app2/test/test_helper.exs create mode 100644 config/config.exs create mode 100644 mix.exs ```

unused-rs works at the root of the umbrella project:

$ git ctags
$ unused-rs -l high --format compact
🧐 Analyzing... [########################################]       9/9      (0s)
App1Test    apps/app1/test/app1_test.exs    Only one occurrence exists
App2Test    apps/app2/test/app2_test.exs    Only one occurrence exists

but it doesn't work when you try the same command in a specific app:

$ cd apps/app2/
$ unused-rs -l high --format compact
Failed to parse tags

Uh oh!

It looks there's an issue with your ctags file; either it doesn't exist, or the formatting is off.

Ensure you've installed Universal Ctags (https://ctags.io/) and re-run it within your application.

Error:
Unable to find ctags file (searched in .git/tags, tags, tmp/tags): No such file or directory (os error 2)

I think that unused-rs should rely in something like git rev-parse --git-dir to discover the path to the .git directory, this should also work when you're inside specific apps in an umbrella project:

$ git rev-parse --git-dir
/tmp/demo/.git
$ ls ../../.git/tags
../../.git/tags

BTW, I noticed the same behavior on unused:

$ pwd
/tmp/demo/apps/app2
$ unused --search rg -l high

There was a problem finding a tags file.

Looked for a 'tags' file in the following directories:

* .git
* tmp
* .

If you're generating a ctags file to a custom location, you can pipe it into unused:
    cat custom/ctags | unused --stdin

You can find out more about Exuberant Ctags here:
    http://ctags.sourceforge.net/

You can read about a good git-based Ctags workflow here:
    http://tbaggery.com/2011/08/08/effortless-ctags-with-git.html
joshuaclayton commented 4 years ago

@milmazz in thinking through this further, I'd be interested in your take on what considerations should be factored in here.

Specifically, tokens across all apps (e.g. app1 and app2) will be present in the tags file being used for identifying tokens within e.g. app2. If app2 references a token once from app1, I think it'd register as unused if there was a single occurrence in the app where unused was run.

This doesn't seem ideal, but I'd defer to you as you'd mentioned running in a child rather than in the root.

milmazz commented 4 years ago

Specifically, tokens across all apps (e.g. app1 and app2) will be present in the tags file being used for identifying tokens within e.g. app2.

Yes, the tags are shared among all the applications in the umbrella app.

If app2 references a token once from app1, I think it'd register as unused if there was a single occurrence in the app where unused was run.

This doesn't seem ideal

Sadly this is the case for mix xref callers at the moment in an umbrella project, I mention this because I tend to use mix xref callers as a way to confirm the unused functions found by unused.

But I agree with you, that behavior is not ideal, I think that unused should consider all the tokens when it's trying to find unused functions, but for presentational purposes, in this case, it should only display unused functions in the current child.

joshuaclayton commented 4 years ago

@milmazz I've got a pass of this working on https://github.com/unused-code/unused_rs/tree/rev-parse.

This only uses git rev-parse --git-dir to calculate the correct path for .git/tags, but I'm wondering now if it also makes sense to do relative path traversal to look for tags and tmp/tags the same way.

So, in this linked branch, it'll join the "rev-parse"d path with tags, resulting in APP_ROOT/.git/tags – I'm wondering if we should also join "../tags" and "../tmp/tags" to ensure we're looking in the right spot (relative to .git) if the tags file doesn't live in .git. Any thoughts?

joshuaclayton commented 4 years ago

@milmazz another note – while this branch does correctly determine the .git path, it does not filter results down to those that occur within the current directory. I'd be interested in seeing some real-world use cases or digging into this further before exploring this, as I'm guessing there will be a fair amount of lift to filter results based on the cwd.

joshuaclayton commented 4 years ago

This should be resolved via 1f0ea1d613f25da980ff70768442a8e569c89dd4; thanks for your patience @milmazz!