Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added disable-previous-line. #91

Merged
merged 4 commits into from
Dec 21, 2018
Merged

Added disable-previous-line. #91

merged 4 commits into from
Dec 21, 2018

Conversation

nventuro
Copy link
Contributor

Similar to solhint-disable-next-line, previous-line disables linter errors on the line preceding the comment directive.

This is very useful when disabling no-empty-blocks errors, since it lets you put the directive inside the (empty) block, and not before it, where there may be comments (e.g. if it is a function).

Some examples from OpenZeppelin:

/**
 * @dev Validation of an executed purchase. Observe state and use revert statements to undo rollback when valid
 * conditions are not met.
 * @param beneficiary Address performing the token purchase
 * @param weiAmount Value in wei involved in the purchase
 */
function _postValidatePurchase(address beneficiary, uint256 weiAmount) internal view {
    // solhint-disable-previous-line no-empty-blocks
}
/**
 * @dev This contract should only be used by extending from it, deploying it as a base contract is disabled.
 */
constructor () internal {
  // solhint-disable-previous-line no-empty-blocks
} 

test/comment-directives.js Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Dec 20, 2018

Coverage Status

Coverage increased (+0.002%) to 99.529% when pulling 99efbf9 on nventuro:feat/disable-previous-line into f389547 on protofire:master.

Copy link
Contributor

@fvictorio fvictorio left a comment

Choose a reason for hiding this comment

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

Looks good. I added an extra test for using this rule with a multi-line comment.

@nventuro
Copy link
Contributor Author

@fvictorio note that these tests are affected by #92: we may want to merge that one first and then fix the asserts.

@nventuro
Copy link
Contributor Author

Merged #92 into this, should be ready now.

@fvictorio fvictorio merged commit fb52933 into protofire:master Dec 21, 2018
@nventuro nventuro deleted the feat/disable-previous-line branch December 21, 2018 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants