Skip to content
This repository has been archived by the owner on Jan 19, 2019. It is now read-only.

Added rule for "_" suffix for internal variables #31

Conversation

viquezclaudio
Copy link

New rule for "_" suffix for internal variables: variables with no visibility qualifier or private or internal variables. Does not apply for variables in a function

Copy link
Contributor

@come-maiz come-maiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @viquezclaudio!
This is very close to be green.

Please add tests for internal and private explicit declarations.
I'm inclined to reject public state variables with underscore prefix, but maybe later. What do you think about this?

And about the rule name, I think it should either be called internal-state-underscore-suffix, or receive the suffix as an argument, with _ being the default.

README.md Outdated
@@ -41,6 +41,9 @@ In the .soliumrc.json file, add:
],
"zeppelin/no-unused-imports": [
"warning"
],
"zeppelin/internal-state-suffix":[
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a space after the colon.

if (emitted.exit) {
return;
}
emitted.node.params.forEach(param => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this case, there are no params. So remove this forEach loop.
We just need to check:
emitted.node.name.charAt(emitted.node.name.length - 1) !== "_"

The function inspectStateVariableDeclaration will be called for each declaration, so the loop is already happening, but one level higher.

errors.should.be.instanceof(Array);
errors.length.should.equal(1);
errors[0].message.should.equal(
"'internal_no_underscore_suffix' does not have an underscore suffix.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message you defined says "underscore AS suffix"

@viquezclaudio viquezclaudio force-pushed the rule/internal_state_suffix branch 2 times, most recently from 64b94ae to 86ec63a Compare August 31, 2018 14:17
@viquezclaudio
Copy link
Author

viquezclaudio commented Aug 31, 2018

@ElOpio You are right, I will add tests for explicit internal and private declarations.

Regarding your question, do you mean creating a rule to reject public variables with underscore suffix ?

@come-maiz
Copy link
Contributor

Thanks @viquezclaudio. With your help and after a few iterations with this rules, we realized that if we required all state variables to be private, we could prefix them with an underscore (like on python), and there will be no conflict with function parameters that will be without underscore. I'm closing this one in favor of #32. I copied your code with a few tweaks, so it would be nice if you can review it.

@come-maiz come-maiz closed this Oct 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants