Skip to content

Commit

Permalink
Merge pull request #412 from protofire/feat/add-github-action-to-test…
Browse files Browse the repository at this point in the history
…-repo

feat: removed travis and added automated tests via github workflow
  • Loading branch information
fvictorio authored and dbale-altoros committed Mar 6, 2023
2 parents d97d0fb + b4c4ce9 commit cde1bc6
Show file tree
Hide file tree
Showing 11 changed files with 171 additions and 88 deletions.
2 changes: 2 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# prevent github actions to checkout files with crlf line endings
* -text
11 changes: 9 additions & 2 deletions .github/workflows/E2E.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,28 @@ jobs:
name: Test on Linux with Node ${{ matrix.node }}
strategy:
matrix:
node: [14, 16]
node: [14, 16, 18]

steps:
- uses: actions/setup-node@v1
with:
node-version: ${{ matrix.node }}
- uses: actions/checkout@v2
- uses: actions/checkout@v3

- name: install
run: npm install --include=dev

- name: lint
run: npm run lint

- name: generate-docs
run: npm run docs

- name: pack
run: npm pack

- name: install-globally
run: npm i -g solhint*tgz

- name: run-e2e-tests
run: cd e2e && npm install && npm test
69 changes: 69 additions & 0 deletions .github/workflows/TESTS.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# This workflow will do a clean installation of node dependencies, cache/restore them, build the source code and run tests across different versions of node
# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-nodejs

name: TESTS

on:
push:
branches: [ "master" ]
pull_request:
branches: [ "master" ]

jobs:
test_on_linux:
name: Run linter and tests on Linux
runs-on: ubuntu-latest

strategy:
matrix:
node-version: [14, 16, 18]

steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node-version }}

- name: Install dependencies
run: npm ci

- name: Run linter
run: npm run lint
- name: Run Tests
run: npm test

test_on_windows:
name: Run linter and tests on Windows
runs-on: windows-latest

steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 18

- name: Install dependencies
run: npm ci

- name: Run linter
run: npm run lint
- name: Run Tests
run: npm test

test_on_macos:
name: Run linter and tests on MacOS
runs-on: macos-latest

steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 18

- name: Install dependencies
run: npm ci

- name: Run linter
run: npm run lint
- name: Run Tests
run: npm test
14 changes: 0 additions & 14 deletions .travis.yml

This file was deleted.

1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
[![Donate with Ethereum](https://en.cryptobadges.io/badge/micro/0xe8cdf02efd8ab0a490d7b2cb13553389c9bc932e)](https://en.cryptobadges.io/donate/0xe8cdf02efd8ab0a490d7b2cb13553389c9bc932e)

[![Gitter chat](https://badges.gitter.im/gitterHQ/gitter.svg)](https://gitter.im/solhint/Lobby)
[![Build Status](https://travis-ci.org/protofire/solhint.svg?branch=master)](https://travis-ci.org/protofire/solhint)
[![NPM version](https://badge.fury.io/js/solhint.svg)](https://npmjs.org/package/solhint)
[![Coverage Status](https://coveralls.io/repos/github/protofire/solhint/badge.svg?branch=master)](
https://coveralls.io/github/protofire/solhint?branch=master)
Expand Down
36 changes: 30 additions & 6 deletions docs/rules/naming/named-parameters-mapping.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,28 +39,52 @@ mapping(string name => uint256 balance) public users;
mapping(address owner => mapping(address token => uint256 balance)) public tokenBalances;
```

#### Main key of mapping is enforced. On nested mappings other naming are not neccesary

```solidity
mapping(address owner => mapping(address => uint256)) public tokenBalances;
```

#### Main key of the parent mapping is enforced. No naming in nested mapping uint256

```solidity
mapping(address owner => mapping(address token => uint256)) public tokenBalances;
```

#### Main key of the parent mapping is enforced. No naming in nested mapping address

```solidity
mapping(address owner => mapping(address => uint256 balance)) public tokenBalances;
```

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

#### No naming in regular mapping
#### No naming at all in regular mapping

```solidity
mapping(address => uint256)) public tokenBalances;
```

#### No naming in nested mapping
#### Missing any variable name in regular mapping uint256

```solidity
mapping(address token => uint256)) public tokenBalances;
```

#### Missing any variable name in regular mapping address

```solidity
mapping(address => mapping(address => uint256)) public tokenBalances;
mapping(address => uint256 balance)) public tokenBalances;
```

#### No complete naming in nested mapping. Missing main key and value
#### No MAIN KEY naming in nested mapping. Other naming are not enforced

```solidity
mapping(address => mapping(address token => uint256)) public tokenBalances;
mapping(address => mapping(address token => uint256 balance)) public tokenBalances;
```

## Version
This rule is introduced in the latest version.
This rule was introduced in [Solhint 3.4.0](https://github.com/protofire/solhint/tree/v3.4.0)

## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/naming/named-parameters-mapping.js)
Expand Down
40 changes: 25 additions & 15 deletions lib/rules/naming/named-parameters-mapping.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,38 @@ const meta = {
'To enter owner token balance, the main key "owner" enters another mapping which its key is "token" to get its "balance"',
code: 'mapping(address owner => mapping(address token => uint256 balance)) public tokenBalances;',
},
{
description:
'Main key of mapping is enforced. On nested mappings other naming are not neccesary',
code: 'mapping(address owner => mapping(address => uint256)) public tokenBalances;',
},
{
description:
'Main key of the parent mapping is enforced. No naming in nested mapping uint256',
code: 'mapping(address owner => mapping(address token => uint256)) public tokenBalances;',
},
{
description:
'Main key of the parent mapping is enforced. No naming in nested mapping address',
code: 'mapping(address owner => mapping(address => uint256 balance)) public tokenBalances;',
},
],
bad: [
{
description: 'No naming in regular mapping ',
description: 'No naming at all in regular mapping ',
code: 'mapping(address => uint256)) public tokenBalances;',
},
{
description: 'No naming in nested mapping ',
code: 'mapping(address => mapping(address => uint256)) public tokenBalances;',
description: 'Missing any variable name in regular mapping uint256',
code: 'mapping(address token => uint256)) public tokenBalances;',
},
{
description: 'Missing any variable name in regular mapping address',
code: 'mapping(address => uint256 balance)) public tokenBalances;',
},
{
description: 'No complete naming in nested mapping. Missing main key and value ',
code: 'mapping(address => mapping(address token => uint256)) public tokenBalances;',
description: 'No MAIN KEY naming in nested mapping. Other naming are not enforced',
code: 'mapping(address => mapping(address token => uint256 balance)) public tokenBalances;',
},
],
},
Expand Down Expand Up @@ -63,32 +82,23 @@ class NamedParametersMapping extends BaseChecker {

checkNameOnMapping(variable, isNested) {
let mainKeyName
let nestedKeyName
let valueName

if (isNested) {
mainKeyName = variable.typeName.keyName ? variable.typeName.keyName.name : null
nestedKeyName = variable.typeName.valueType.keyName
? variable.typeName.valueType.keyName.name
: null
valueName = variable.typeName.valueType.valueName
? variable.typeName.valueType.valueName.name
: null
} else {
mainKeyName = variable.typeName.keyName ? variable.typeName.keyName.name : null
nestedKeyName = null
valueName = variable.typeName.valueName ? variable.typeName.valueName.name : null
}

if (!mainKeyName) {
this.report(variable, 'Main key')
}

if (!nestedKeyName && isNested) {
this.report(variable, 'Nested key')
}

if (!valueName) {
if (!valueName && !isNested) {
this.report(variable, 'Value')
}
}
Expand Down
3 changes: 2 additions & 1 deletion test/common/utils.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
const fs = require('fs')
const os = require('os')
const path = require('path')

function tmpFilePath() {
const tempDirPath = os.tmpdir()
return `${tempDirPath}/test.sol`
return path.resolve(tempDirPath, 'test.sol')
}

function storeAsFile(code) {
Expand Down
Loading

0 comments on commit cde1bc6

Please sign in to comment.