Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/rules/best-practices/use-natspec.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ This rule accepts an array of options:
```

### Notes
- For `internal` or `private` function this rule checks natspec as configured, only if there are tags present. If not, function is skipped.
- For `external` or `public` function this rule checks natspec as specified in the config.
- If a function or return value has unnamed parameters (e.g. `function foo(uint256)`), the rule only checks the number of `@param` or `@return` tags, not their names.
- If a function or variable has `@inheritdoc`, the rule skips the validation.
- The rule supports both `///` and `/** */` style NatSpec comments.
Expand Down
27 changes: 20 additions & 7 deletions lib/rules/best-practices/use-natspec.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,12 @@ const meta = {
],
},
notes: [
{
note: 'For `internal` or `private` function this rule checks natspec as configured, only if there are tags present. If not, function is skipped.',
},
{
note: 'For `external` or `public` function this rule checks natspec as specified in the config.',
},
{
note: 'If a function or return value has unnamed parameters (e.g. `function foo(uint256)`), the rule only checks the number of `@param` or `@return` tags, not their names.',
},
Expand Down Expand Up @@ -253,7 +259,7 @@ class UseNatspecChecker extends BaseChecker {
if (!hasParameters(node)) tags.splice(tags.indexOf('param'), 1)
if (!hasReturn(node)) tags.splice(tags.indexOf('return'), 1)

this.checkNode(node, 'function', tags, isPublicLike)
this.checkNode(node, 'function', tags, isPublicLike, isPublicLike)
}

EventDefinition(node) {
Expand All @@ -267,17 +273,24 @@ class UseNatspecChecker extends BaseChecker {
this.checkNode(node, 'variable', NATSPEC_TARGETS.variable, true)
}

checkNode(node, type, overrideTags, requireNoticeIfPublic = false) {
checkNode(node, type, overrideTags, requireNoticeIfPublic, isPublicLike = false) {
const name = node.name || (node.id && node.id.name) || '<anonymous>'
const tagsRequired = overrideTags || NATSPEC_TARGETS[type] || []
const startOffset = node.range[0]
const comments = getLeadingNatSpecComments(startOffset, this.tokens)
const tags = extractNatSpecTags(comments)

// console.log('\n>>> CHECKING', type, name)
// console.log('COMMENTS:', comments)
// console.log('TAGS:', tags)
console.log('isPublicLike :>> ', isPublicLike)
console.log('comments.length :>> ', comments.length)
console.log('type :>> ', type)

// check the function is not publicLike and has no tags, avoid checking
// if it is internal or private and has natspec comments, check them
if (!isPublicLike && comments.length === 0 && type === 'function') return

// get the tags from comments
const tags = extractNatSpecTags(comments)

// check the function is publicLike and has inheritdoc
const hasInheritdoc = tags.includes('inheritdoc')
if (requireNoticeIfPublic && hasInheritdoc) return

Expand Down Expand Up @@ -369,7 +382,7 @@ function getLeadingNatSpecComments(startOffset, tokens) {
comments.unshift(value)
}

// Si encontramos código real (no comentario ni puntuación), cortamos
// If there is real code (not comment or punctuation), break
if (!value || (!value.startsWith('///') && !value.startsWith('/**'))) {
if (token.type !== 'Punctuator') break
}
Expand Down
89 changes: 89 additions & 0 deletions test/rules/best-practices/use-natspec.js
Original file line number Diff line number Diff line change
Expand Up @@ -543,5 +543,94 @@ describe('Linter - use-natspec', () => {
"Mismatch in @return count for function 'doSomething'. Expected: 1, Found: 0"
)
})

it('should ignore internal and private functions if there are no tags', () => {
const code = `
contract HalfIgnored {

function doSomething1(uint256 amount) internal{}

function doSomething2(uint256 amount) private{}

/// @dev documented internal function
function doSomething2(uint256 amount) private{}
}
`

const report = linter.processStr(code, {
rules: {
'use-natspec': [
'error',
{
title: {
enabled: false,
},
author: {
enabled: false,
},
notice: {
enabled: true,
ignore: {
contract: ['HalfIgnored'],
},
},
param: {
enabled: true,
},
},
],
},
})
assertNoErrors(report)
})

it('should check internal and private functions if there are tags', () => {
const code = `
contract HalfIgnored {

/// @notice does something
/// @param amount1 the amount
function doSomething1(uint256 amount) internal returns(uint256){}
}
`

const report = linter.processStr(code, {
rules: {
'use-natspec': [
'error',
{
title: {
enabled: false,
},
author: {
enabled: false,
},
notice: {
enabled: true,
ignore: {
contract: ['HalfIgnored'],
},
},
param: {
enabled: true,
},
return: {
enabled: true,
},
},
],
},
})
assertErrorCount(report, 3)
assert.ok(
report.reports[0].message,
"Mismatch in @param names for function 'doSomething1'. Expected: [amount], Found: [amount1]"
)
assert.ok(report.reports[1].message, "Missing @return tag in function 'doSomething1'")
assert.ok(
report.reports[2].message,
"Mismatch in @return count for function 'doSomething'. Expected: 1, Found: 0"
)
})
})
})
Loading