lint rule for visibility annotations

[jessegreenberg]

I made a lint rule to catch missing visibility annotations for method declarations. Would people find this useful generally or annoying? We could probably also have a lint rule to catch missing visibility annotations on instance variables.

[jessegreenberg]

Checked in but not enabled. When run with lint-everything, it is catching 752 methods that do not have visibility annotations.

[samreid]

Looks great, we should enable this and address existing deficiencies.

[zepumph]

I think we should turn it on! Thanks for implementing it.

[zepumph]

To turn it on, go to chipper/eslint/rules/.eslintrc.js and uncomment the visibility-annotation rule.

UPDATE FROM @samreid: Then run with --disable-eslint-cache

UPDATE FROM @pixelzoom:

• The correct file to edit is chipper/eslint/.eslintrc.js
• The command to run is grunt lint --disable-eslint-cache

Devs will take on their own sims, and we may make a chip-away list.

[samreid]

Currently at 812 errors for that lint rule (from lint-everything).

UPDATE: This looks like some other lint errors crept in, may not be accurate.

[pixelzoom]

Self assigning to run this lint rule on the sims that I'm responsible for.

[jbphet]

I've addressed this in the sim and common repos for which I believe I'm responsible. I'm not sure why, but I'm the only one who was assigned to this issue. I'm going to pass it off to @jessegreenberg because there seem to be a number of a11y-related instances of this problem in inverse-square-law-common and tappi. Other than that, there are just a handful that we can probably tackle in the meeting when we turn on the lint rule.

@jessegreenberg - you're welcome to assign to whoever owns the most instance of these errors once you're done.

[pixelzoom]

@jbphet you were the only person assigned because you were the last to have items to check off in the checklist found in #932 (comment). Can all of those now be checked off?

[jbphet]

@pixelzoom - Thanks for pointing that out - I've checked off the items that I addressed. There are still 24 errors occurring for grunt lint-everything.

[pixelzoom]

I took care of a few of the remaining errors, which all appear to be new problems that have crept in.

Here's what's left:

• inverse-square-law-common @jessegreenberg
• ratio-and-proportion @zepumph
• scenery a11y @jessegreenberg
• studio @zepumph @chrisklus
• tappi @jessegreenberg

[pixelzoom]

I addressed the new errors noted in #932 (comment), then enabled the visibility-annotations rule, so that we don't introduce more errors.

I posted this PSA to the Slack developer channel:

24 new cases of missing visibility annotations have snuck in. So I addressed all of them, and then enabled the visibility-annotation lint rule. Please pull chipper so that you have this new rule, otherwise you run the risk of introducing a new lint error that will fail in CT. More details at #932.

[pixelzoom]

@jessegreenberg review the changes that I made to his code. @zepumph will review if he wants to. Consensus on Slack was to close this issue.