Use cache object

[russHyde]

• [x] Add class Cache
• [x] Convert functions in cache.R to methods on Cache
• [x] Push logic re 'whether to cache' lints down from lint() to Cache-methods
• [ ] Split cache.R into cache-class.R and cache-methods.R
• [ ] Add integration tests on lint that require the use of a cache
• [ ] Change test-cache.R:
  • [ ] pass in Cache object
  • [ ] handle NullCache (nothing should be saved/retrieved if the user doesn't want to use a cache)

Adds a new S3 class Cache and converts some pre-existing cacheing functions to be methods on this class.

There was multiple checks of the form if(cache_exists) within the lint() function prior to this PR; this made reading the function complicated.

These checks are now handled by the Cache class and it's methods: if called, cache_lint, save_cache, and cache_file will only modify the cache when relevant. That is, if the user has called lint(some_file, cache = FALSE) the *_cache and cache_* methods will still be called, but they will not create/modify a cache file.

This reduces the cyclomatic complexity of lint() from 33 to 23 (see #653 ) (further improvements to the readability of lint() could come from moving the linter definition and validation steps into helper functions and from splitting the lint() function into an input-validation function and a separate lint-runner function that works on prevalidated input)

[russHyde]

This seems reasonable (I'll change *.default to *.environment and then push the *.environment code into the *.Cache methods); I wasn't sure what the type of the 'cache' input was in the functions in cache.R and was just trying to implement the idea with the fewest changes to the code. Should I rewrite the other functions in cache.R to use Cache objects as input?

[AshesITR]

That seems the best way to go. Only working with Cache objects in all cache functions makes sense to me.

[AshesITR]

WDYT about defining the generics for .FileCache and .NullCache instead? Cache(...) would then create either a structure(list(), class = c("NullCache", "Cache")) or a structure(list(...), class = c("FileCache", "Cache")) and the .NullCache implementations would be no-ops whereas the .FileCache implementations would use the info contained in the list to do the requested operations to its underlying environment / file.

[russHyde]

Thanks, yes that was the plan. The tests would need rewriting before that can be done, so it's not a straightforward refactoring. It will come together soon.

[russHyde]

Thanks for your review of this draft PR @ashesITR. I really don't think I'll be able to get this done with any immediacy. I'll have to rethink the Cache implementation, because the unit tests in test-cache.R are difficult to rewrite.

[AshesITR]

Hi @russHyde. The PR has began to diverge from master but is very valuable I think. Do you think you'll get the chance to work on this in the near future? If so, I can try to resolve the merge conflicts if that helps.

[IndrajeetPatil]

Dear @russHyde, let us know if you want to resolve the merge conflicts here before you continue, or if you want us to take it from here, in case you can no longer work on it. Thanks :)

[MichaelChirico]

Gonna close since this is so far diverged from main / not much apparent appetite for working on it. Feel free to tackle from scratch on current main.