allow inline configuration through comment directives

[sohkai]

There are a number of cases where one or more functions in a contract may be in a different order than the recommended visibility order due to logical grouping; one example is an initialize() being next to a constructor rather than after a large number of other functions.

It would be nice to have some way to configure an ignores list, or even ignore a function from the rule if the preceding line is /* solium-disable-next-line function-order */ (this sounds hard to get right though).

[duaraghav8]

@sohkai Correct me if I'm wrong. My understanding is that you would like to force function-order to NOT report a particular function (like initialize()) if it is out of order, right?

If that's what you want, it is already possible. For eg, I just took your ACL.sol from dev to see which functions were being reported by function-order:

$ solium -f acl.sol

acl.sol
  77:4     error    Functions should be in order: constructor, fallback, external, public, internal, private    function-order
  90:4     error    Functions should be in order: constructor, fallback, external, public, internal, private    function-order
  119:4    error    Functions should be in order: constructor, fallback, external, public, internal, private    function-order
  132:4    error    Functions should be in order: constructor, fallback, external, public, internal, private    function-order
  144:4    error    Functions should be in order: constructor, fallback, external, public, internal, private    function-order
  158:4    error    Functions should be in order: constructor, fallback, external, public, internal, private    function-order
  170:4    error    Functions should be in order: constructor, fallback, external, public, internal, private    function-order

✖ 7 errors found.

Now if I add // solium-disable-next-line function-order right above the line 77 (declaration of createPermission()), then re-run solium, this function is no longer reported.

Does that solve your problem?

[sohkai]

@duaraghav8 Slightly different; in the case of the ACL, I'd like to remove the offenders (initialize() and grantPermissionP()) entirely from the rule rather than the functions that are in the right order.

I'm thinking of it as subtracting those functions from the set of functions that are used to evaluate the rule.

[duaraghav8]

I think I got the problem. Say you have functions

A private()
constructor()
B public()

and you'd like to keep A on top, which causes the rule to report the remaining 2 functions because the rule thinks they're not in the right order. The effect of ignoring function A means it is invisible to function-order, so the rest of the functions are seen to be in the right order, so no errors are reported.

Is that correct?

[sohkai]

Yes, that's the idea :pray:!

[duaraghav8]

Here's the configuration that I'm planning:

{
    "rules": {
        "function-order": [
            "error",
            {
                "ignore": {
                    "constructorFunc": true,
                    "fallbackFunc": true,
                    "functions": ["foo", "myFunc"],
                    "visibilities": ["private"]
                }
            }
        ]
    }
}

By default, the arrays are empty and constructor is set to false, which means NO functions are ignored. If you specify a visibility in visibilities, all functions of that vis. are ignored.

Update: I'm using constructorFunc because constructor is also a native property of all JS objects, causing conflicts with solium's schema validator.

---

fix for this is on develop - https://github.com/duaraghav8/Solium/commit/537f8c967b8c3bf02a40e145042e221d096a58e3

will be available in next release

[sohkai]

Not entirely sure how solium handles option setting via comments in files (can it?), but it'd be great if this were supported as well on a per-file basis that way.

[duaraghav8]

Currently not possible, but I can see how per-file configuration is highly relevant in this case. I'll try and add this before next release

[duaraghav8]

The original feature request for adding ignore option in function-order has been solved in v1.2.0. The issue will still remain open because its topic has changed.

This linter is now being published simultaneously to npm packages solium and ethlint. Although not mandatory, I highly recommend that you move to ethlint.

See changelog

[duaraghav8]

Use case A user has a soliumrc configuration file with which they lint the project. However, they'd like to lint some files with different configurations. Currently, this cannot be achieved and the user has to configure linter to ignore that file. This feature will allow the user to write an alternative linter configuration inside the .sol file. When ethlint runs over this file, the configuration written in it is used instead of soliumrc's.

Thoughts Feature can be invoked using the soliumrc comment directive, i.e., a solidity comment starting with the word soliumrc, followed by a JSON object declaring linter configuration to apply to current file. This object is exactly like the configuration you'd write in .soliumrc.json. Extra whitespace is ignored.

As a best practice, the configuration should be written before any code in the file. Example:

// comment 1
/* comment 2 */

/*
  soliumrc

{
  "quotes": ["error", "double"],
  "security/no-throw": "error",
  "linebreak-style": ["error", "unix"]
}
*/

contract MyContract {
  // ...
}

Json inside comment block seems tedious, which is why I am open to discussion and inputs. But we need to accommodate all config attributes (extends, plugins, rules, etc.) to allow the user to write any sort of configuration for that particular file.

Usage If you have a .soliumrc.json, it gets overridden by inline configuration. If you'd only like to use inline configuration without having any .soliumrc.json, you can use the --no-soliumrc cli option while running the linter.

[sohkai]

YAML could be used instead of JSON as a more friendly serialization format in files, but on some level I'm not sure this will be that useful to most people since they can override settings one by one through existing solium comment directives right now.