Yaml metrics

[ukinimod]

Add Yaml metrics

[bastiandg]

I could only do a partial review of the code, because it's quite extensive. Please click rerequest when you are done with working one my comments. After that I will do a second review round.

1. Your code doesn't abide by the rules you set for yourself (.golangci-lint.yml):

$ golangci-lint run
pkg/metrics/metrics.go:13:23: ptrToRefParam: consider `origin' to be of non-pointer type (gocritic)
func AggregateMetrics(origin, additional *(map[string]Metric)) {
                      ^
cmd/root.go:16:1: don't use `init` function (gochecknoinits)
func init() {
^
pkg/output/csvprinter.go:46:75: string `ansible_project` has 2 occurrences, make it a constant (goconst)
                printAsCsv(metrics, func(it string) bool { return it == "role" || it == "ansible_project" })
                                                                                        ^
pkg/ansible/comments_test.go:10:10: string `test/data/main.yml` has 3 occurrences, make it a constant (goconst)
        path := "test/data/main.yml"
                ^
pkg/ansible/comments.go:16:27: string `.yml` has 3 occurrences, make it a constant (goconst)
        if filepath.Ext(path) == ".yml" {
                                 ^
pkg/ansible/type.go:119:32: string `roles` has 2 occurrences, make it a constant (goconst)
                if fileinfoContent.Name() == "roles" ||
                                             ^
pkg/ansible/type.go:86:100: Using the variable on range scope `fileinfoContent` in function literal (scopelint)
                index := sort.Search(len(necessaryRolePaths), func(i int) bool { return necessaryRolePaths[i] >= fileinfoContent.Name() })
                                                                                                                 ^
cmd/ansible.go:15:25: mnd: Magic number: 1, in <argument> detected (gomnd)
        Args:  cobra.ExactArgs(1),
                               ^
internal/util/fileUtil.go:35:10: mnd: Magic number: 1, in <return> detected (gomnd)
                return 1
                       ^

Fix this, either by consciously changing the rules or by fixing the code.

2. The license and readme aren't reviewed here, which is a shame.

3. Even binary files are parsed by the iac-count tool:

14:07:09 Fri Mar 20 - ~/git/iac-count
bastian@bastiand ✓ $ dd if=/dev/urandom iflag=fullblock bs=1G count=2 > pkg/ansible/test/data/roles/example/handlers/dd.png
2+0 records in
2+0 records out
2147483648 bytes (2.1 GB, 2.0 GiB) copied, 13.2134 s, 163 MB/s
14:07:35 Fri Mar 20 - ~/git/iac-count
bastian@bastiand ✓ $ time ./iac-count ansible pkg/ansible/test/data/roles/example/handlers/
path,type,comment_lines,files,loc,rloc
.,directory,1,3,8391028,9
dd.png,file,,1,8390976,
dd.yaml,file,,1,41,
main.yml,playbook,1,1,11,9

real    0m29.360s
user    0m24.851s
sys     0m8.182s

There should be at least a very simple heuristic approach to skipping those files.

4. Your testing looks very handcrafted and hard to maintain. In go there's a concept called "table based testing". Please change your test case accordingly. Links for more details:

• Golang Table Driven Tests
• Medium: Basic testing patterns in Go
• Helm cli test file

5. The behavior of --skip-dirs is unintuitive. It will only skip top level directories in the checked root directory.

Example

./iac-count ansible --skip-dirs "pkg" pkg

• doesn't skip pkg
• skips pkg/pkg
• doesn't skip pkg/sub/dir/pkg

[bastiandg]

This should terminate the application or at least the function. Don't just ignore the error and continue. I know Ignorance is bliss ;) But this doesn't work in this case.

[ukinimod]

I will rethink the whole error handling in a seperate pr

[ukinimod]

1. I changed the rules of golangci-lint
2. We talked about the license and readme and I made the corresponding changes
3. Only known text files will be checked using their mime type. ATM there is no catch for too large files. I will integrate this later.
4. I tried table driven tests.
5. The feature is deactivated. This will be part of a future pr.

[bastiandg]

The structure looks very good to me. It needs another full-blown review for the details though.

The thing I'm thinking about when I see the very generalized structure is: How do we represent and analyze the specificities of particular languages? Let's talk about it in person.