Documentation, add simple scalar scalar SEXPs, weak warnings, cleanup #7
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…/resources` Move snapshots into `src/test` so they're closer to the resources they get generated from and are easier to find. Combine `src/test/R` and `src/test/resources` because the distinction is confusing and adds extra complexity we don't really need. Also add a few comments to clarify how the resources and snapshots are used.
…e place we can just pattern match in the one place if we ever use it (I believe it's `set_tag` or something in the interpreter)
I revealed these SEXPs and not the other variants because these are literals we may see compiled. The other variants would probably never be used. Also I think (hope because SEXPs are ugly) we'll use regular objects instead of SEXPs as much as possible, especially when generating native code. But I can see cases where we need to statically type the simple scalar SEXPs.
And where it isn't reasonable (e.g. too obvious or a GNU-R named thing), suppress the documentation warning. Also does various code cleanup discovered while documenting (e.g. exceptions thrown by specific operations should have package-private constructors).
… that IntelliJ default doesn't. These may be further discussed and changed, but it makes sense for everyone to have the same inspections: - Weak warning for missing Javadoc on public declarations - Weak warning for redundant local variable types (`String foo = "bar"`) - Weak warnings for unnecessary qualifiers (e.g. `this.foo` instead of `foo`)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The main change is that I documented every public method, except where unreasonable (e.g. obvious cases), in which case I suppressed the documentation warning.
Also created specific classes for simple-scalar SEXPs, since they are AST nodes I suspect we'll sometime want to pattern-match on them directly (but if not, I just exposed the package-private class).
Added weak warnings for missing Javadoc on public declarations, and a couple other things that IntelliJ reports but doesn't warn for some reason: redundant local variable types (
String foo = "bar") and unnecessary qualifiers (this.fooinstead offoo). These should be automatically available to anyone using IntelliJ, but if not, they are the "project default" inspections. We may want to discuss and change other inspections, IntelliJ has a lot of them, although I think it's best to for the most part stick with the defaults (a lot of the disabled ones are really pedantic and maybe add a lot of IDE overhead).Lastly, various bugfixes and cleanup.