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

I316 quotes inside quotes #485

Merged
merged 3 commits into from
Aug 16, 2023
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: 1 addition & 1 deletion docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ title: "Rule Index of Solhint"
| Rule Id | Error | Recommended | Deprecated |
| --------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------- | ------------ | ---------- |
| [comprehensive-interface](./rules/miscellaneous/comprehensive-interface.md) | Check that all public or external functions are override. This is iseful to make sure that the whole API is extracted in an interface. | | |
| [quotes](./rules/miscellaneous/quotes.md) | Use double quotes for string literals. Values must be 'single' or 'double'. | $~~~~~~~~$✔️ | |
| [quotes](./rules/miscellaneous/quotes.md) | Enforces the use of double or simple quotes as configured for string literals. Values must be 'single' or 'double'. | $~~~~~~~~$✔️ | |


## Style Guide Rules
Expand Down
46 changes: 43 additions & 3 deletions docs/rules/miscellaneous/quotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ title: "quotes | Solhint"


## Description
Use double quotes for string literals. Values must be 'single' or 'double'.
Enforces the use of double or simple quotes as configured for string literals. Values must be 'single' or 'double'.

## Options
This rule accepts an array of options:
Expand All @@ -32,11 +32,13 @@ This rule accepts an array of options:
}
```

### Notes
- This rule allows to put a double quote inside single quote string and viceversa

## Examples
### 👍 Examples of **correct** code for this rule

#### String with double quotes
#### Configured with double quotes

```solidity

Expand All @@ -49,9 +51,47 @@ This rule accepts an array of options:

```

#### Configured with single quotes

```solidity

pragma solidity 0.4.4;


contract A {
string private a = 'test';
}

```

#### Configured with double quotes

```solidity
string private constant STR = "You shall 'pass' !";
```

#### Configured with single quotes

```solidity
string private constant STR = 'You shall "pass" !';
```

### 👎 Examples of **incorrect** code for this rule

#### String with single quotes
#### Configured with single quotes

```solidity

pragma solidity 0.4.4;


contract A {
string private a = "test";
}

```

#### Configured with double quotes

```solidity

Expand Down
2 changes: 1 addition & 1 deletion docs/rules/naming/foundry-test-functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ This rule accepts an array of options:
- Supported Regex: ```test(Fork)?(Fuzz)?(Fail)?_(Revert(If_|When_){1})?\w{1,}```
- This rule should be executed in a separate folder with a separate .solhint.json => ```solhint --config .solhint.json testFolder/**/*.sol```
- This rule applies only to `external` and `public` functions
- This rule skips the `setUp()` function
- This rule skips the `setUp()` function by default

## Examples
### 👍 Examples of **correct** code for this rule
Expand Down
16 changes: 13 additions & 3 deletions lib/common/identifier-naming.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,18 @@ module.exports = {
return text && text[0] === '_'
},

isNotFoundryTestCase(text) {
const regex = /^test(Fork)?(Fuzz)?(Fail)?_(Revert(If_|When_){1})?\w{1,}$/
return !match(text, regex)
isFoundryTestCase(text) {
// this one checks CamelCase after test keyword
// const regexTest = /^test(Fork)?(Fuzz)?(Fail)?(_)?[A-Z](Revert(If_|When_){1})?\w{1,}$/

const regexTest = /^test(Fork)?(Fuzz)?(Fail)?(_)?(Revert(If_|When_){1})?\w{1,}$/
const matchRegexTest = match(text, regexTest)

// this one checks CamelCase after test keyword
// const regexInvariant = /^(invariant|statefulFuzz)(_)?[A-Z]\w{1,}$/
const regexInvariant = /^(invariant|statefulFuzz)(_)?\w{1,}$/
const matchRegexInvariant = match(text, regexInvariant)

return matchRegexTest || matchRegexInvariant
},
}
28 changes: 24 additions & 4 deletions lib/rules/miscellaneous/quotes.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const meta = {
type: 'miscellaneous',

docs: {
description: `Use double quotes for string literals. Values must be 'single' or 'double'.`,
description: `Enforces the use of double or simple quotes as configured for string literals. Values must be 'single' or 'double'.`,
category: 'Miscellaneous',
options: [
{
Expand All @@ -24,17 +24,38 @@ const meta = {
examples: {
good: [
{
description: 'String with double quotes',
description: 'Configured with double quotes',
code: require('../../../test/fixtures/miscellaneous/string-with-double-quotes'),
},
{
description: 'Configured with single quotes',
code: require('../../../test/fixtures/miscellaneous/string-with-single-quotes'),
},
{
description: 'Configured with double quotes',
code: 'string private constant STR = "You shall \'pass\' !";',
},
{
description: 'Configured with single quotes',
code: 'string private constant STR = \'You shall "pass" !\';',
},
],
bad: [
{
description: 'String with single quotes',
description: 'Configured with single quotes',
code: require('../../../test/fixtures/miscellaneous/string-with-double-quotes'),
},
{
description: 'Configured with double quotes',
code: require('../../../test/fixtures/miscellaneous/string-with-single-quotes'),
},
],
},
notes: [
{
note: 'This rule allows to put a double quote inside single quote string and viceversa',
},
],
},

isDefault: false,
Expand Down Expand Up @@ -64,7 +85,6 @@ class QuotesChecker extends BaseChecker {
token.loc.start.line === node.loc.start.line &&
token.loc.start.column === node.loc.start.column
)

if (token && !this.alreadyVisited(token)) {
this.addVisitedNode(token)
this.validateQuotes(token)
Expand Down
4 changes: 2 additions & 2 deletions lib/rules/naming/foundry-test-functions.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const meta = {
note: 'This rule applies only to `external` and `public` functions',
},
{
note: 'This rule skips the `setUp()` function',
note: 'This rule skips the `setUp()` function by default',
},
],
},
Expand All @@ -79,7 +79,7 @@ class FoundryTestFunctionsChecker extends BaseChecker {
!this.searchInArray(this.skippedFunctions, node.name) &&
(node.visibility === 'public' || node.visibility === 'external')
) {
if (naming.isNotFoundryTestCase(node.name)) {
if (!naming.isFoundryTestCase(node.name)) {
this.error(node, `Function ${node.name}() must match Foundry test naming convention`)
}
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "solhint",
"version": "3.6.1",
"version": "3.6.2",
"description": "Solidity Code Linter",
"main": "lib/index.js",
"keywords": [
Expand Down
30 changes: 30 additions & 0 deletions test/rules/miscellaneous/quotes.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,4 +90,34 @@ describe('Linter - quotes', () => {

assertErrorCount(reports[0], 1)
})

describe('Double quotes inside single and viceversa', () => {
it('should not raise error when configured as single and there are double quotes inside', function test() {
const code = contractWith('string private constant STR = \'You shall "pass" !\';')

const report = linter.processStr(code, { rules: { quotes: ['error', 'single'] } })
assertNoErrors(report)
})

it('Should raise error when configured as single and string is with double quotes despite content', function test() {
const code = contractWith('string private constant STR = "You shall \'NOT\' pass";')

const report = linter.processStr(code, { rules: { quotes: ['error', 'single'] } })
assertErrorCount(report, 1)
})

it('should not raise error when configured as double and there are single quotes inside', function test() {
const code = contractWith('string private constant STR = "You shall \'pass\' !";')

const report = linter.processStr(code, { rules: { quotes: ['error', 'double'] } })
assertNoErrors(report)
})

it('Should raise error when configured as double and string is with single quotes despite content', function test() {
const code = contractWith('string private constant STR = \'You shall "pass" !\';')

const report = linter.processStr(code, { rules: { quotes: ['error', 'double'] } })
assertErrorCount(report, 1)
})
})
})
11 changes: 9 additions & 2 deletions test/rules/naming/foundry-test-functions.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const contractWith = require('../../common/contract-builder').contractWith
const { assertErrorCount, assertNoErrors, assertErrorMessage } = require('../../common/asserts')

const ALLOWED_FUNCTION_NAMES = [
'test',
'test_',
'testFork_',
'testFuzz_',
Expand All @@ -20,9 +21,14 @@ const ALLOWED_FUNCTION_NAMES = [
'testFuzz_Revert_',
'testFuzz_If_',
'testFuzz_When_',
'invariant',
'invariant_',
'invariantA',
'statefulFuzz',
'statefulFuzz_',
]

const DISALLOWED_FUNCTION_NAMES = ['Test_', 'Test', 'test', 'testo_', '', 'any', 'setUp', 'other']
const DISALLOWED_FUNCTION_NAMES = ['Test_', 'Test', '', 'any', 'setUp', 'other', '_']

const composeFunctionName = (prefix, name, visibility) =>
'function ' + prefix + name + ' ' + visibility + ' { testNumber = 42; }'
Expand All @@ -33,6 +39,7 @@ describe('Linter - foundry-test-functions', () => {
const functionDefinition = composeFunctionName(prefix, 'FunctionName()', 'public')
const code = contractWith(functionDefinition)

console.log('`code` :>> ', code)
const report = linter.processStr(code, {
rules: { 'foundry-test-functions': ['error', ['setUp', 'finish']] },
})
Expand Down Expand Up @@ -171,7 +178,7 @@ describe('Linter - foundry-test-functions', () => {
)
})

it(`should NOT raise error only for setUp when rule is just on 'error'`, () => {
it(`should NOT raise error only for setUp when rule is just on 'error' (setUp is default)`, () => {
const code = contractWith(`
function setUp() public { testNumber = 42; }
function finish() public { testNumber = 43; }
Expand Down