Does not delete old aliases if all aliases are removed

[jdhenke]

What happened?

If you have aliases in your conjure definition and generated go code, then remove all aliases and regenerate the go code into the same directory, the old aliases.conjure.go file and alias definitions still exist when they should not.

$ cat /tmp/defs.yml
types:
  definitions:
    default-package: com.palantir
    objects:
      SomeObject:
        fields:
          someField: string
      SomeAlias:
  1 types:
        alias: string
$ ~/Downloads/conjure-4.6.1/bin/conjure compile /tmp/defs.yml /tmp/defs.json
$ go run main.go /tmp/defs.json --output /tmp/repro
$ cat /tmp/repro/com/palantir/aliases.conjure.go
// This file was generated by Conjure and should not be manually edited.

package palantir

type SomeAlias string
$ vi /tmp/defs.yml # remove alias
$ cat /tmp/defs.yml
types:
  definitions:
    default-package: com.palantir
    objects:
      SomeObject:
        fields:
          someField: string
$ ~/Downloads/conjure-4.6.1/bin/conjure compile /tmp/defs.yml /tmp/defs.json
$ go run main.go /tmp/defs.json --output /tmp/repro
$ cat /tmp/repro/com/palantir/aliases.conjure.go # THIS IS THE BUG -- these defs should no longer exist
// This file was generated by Conjure and should not be manually edited.

package palantir

type SomeAlias string

What did you want to happen?

conjure-go should delete the aliases.conjure.go if there are no aliases.

NOTE: This may apply to other files that conjure-go writes as well.

[nmiyake]

This is an issue that's somewhat known. The difficulty here is that conjure-go doesn't store state, so knowing that an existing file "should be deleted" isn't trivial.

The location of the output files (like aliases.conjure.go) is determined by the package path relative to the output directory. If a file in a particular package is no longer generated, then conjure-go doesn't know to look for the file.

The only approach I can think of is a process like the following:

When running the generation task:

1. Crawl the entire output directory tree and determines all of the *.conjure.go files that exist
2. Determine all of the *.conjure.go files that are generated by the task
3. Remove all of the "extra" files (*.conjure.go files that were found but not generated by the task)
4. (Maybe?) If removing the *.conjure.go files caused any directories to become empty based on that action, delete the directory, and do this recursively

Possible issues:

1. If the output location is a large directory tree, crawling the tree may take time (for example, if the output was specified as /)
   • This is probably unlikely to occur, as generation targets are usually targeted directories
   • We could also impose some kind of cap on maximum number of directories/depth that can be visited (and bail out with an error or skip the delete operation if this is exceeded)
2. If the output directory contains symlinks and they are followed, crawling the tree may take time
   • Can prevent by implementing in a manner that does not follow symlinks
3. If multiple separate Conjure tasks/definitions generate into the same output directory, running the task for one definition will delete the output created/managed by another definition
   • I don't believe that this is a common scenario (or one we should even support), but it is a possibility

These possibles issues are probably much rarer than the common scenario outlined in this issue, so not opposed to implementing the fix. However, I think we may want to add a flag that allows this behavior to be disabled as well.