SIP for clarity coding standards

[LNow]

I believe that as a community we should have clear coding standards and guidelines that we want to be followed by developers when they work on clarity contracts.

I drafted something like that for Syvita guild, and we could use it as a starting point.

Naming conventions

Using language reserved keywords (data types, function names and keywords) in any other context than they were design for is prohibited. For example:

;; BAD
(define-map MyMap
  { block-height: uint }
  { len: uint}
)

;; GOOD
(define-map MyMap
  { stx-block-height: uint }
  { length: uint}
)

Constants

Constants have to be declared in all upper case with underscore separators. For example:

(define-constant CONTRACT_OWNER tx-sender)
(define-constant ERR_UNAUTHORIZED (err "Unauthorized!"))

Local and persisted variables

Local and persisted variables have to be declared in camelCase. For example:

(define-data-var blockSize uint u128)
(let
    (
        (firstVar (+ 1 10))
        (secondVar (pow u2 u8))
    )
    (ok (call-function firstVar secondVar))
)

Maps

Maps have to be declared in PascaCase. Key tuple definition and Map tuple definition should be declared in separate lines and follow tuples standard. For example:

(define-map UserRoles
    {user: principal}
    {
        roles: uint,
        isActive: bool
    }
)

To save space and reduce execution costs both key and value can be declared as unnamed data types. For example:

(define-map UserId
    principal
    uint
)

(define-map UserRoles
    principal
    {
        roles: uint,
        isActive: bool
    }
)

Tuples

Tuples have to be declared using curlybracket notation. Each tuple key have to be defined in separate line and follow local and persisted variables standard. For example:

{
    user: tx-sender,
    roles: u1,
    group: "A",
    membersCount: 123
}

Functions

Functions have to be declared in all lower case with dash separators. Function parameters should follow local and persisted variable standards For example:

(define-read-only (is-root (userName (buff 50)))
    (ok true)
)

(define-read-only (is-member (userId uint) (groupId uint))
    (false)
)

Errors

Custom errors should be defined as constants unsigned integers greater than 100, to be easily distinguishable from errors raised by build in functions. Name of each error should start with ERR_ followed by clear description of error. For example:

(define-constant ERR_UNAUTHORIZED u401)

(define-public (some-function)
  (asserts! (is-eq tx-sender contract-caller)
    (err ERR_UNAUTHORIZED)
  )
)

Closing parenthesis

Parenthesis should be closed in the same line if line is very short or at the same level as they were opened if what is inside parenthesis is more complex. For example

;; BAD
(define-public (do-something-cool (userId uint))
  (let
   ((firstVar (+ 1 10))
    (secondVar (pow u2 u8)))
    (ok (call-function firstVar secondVar))))

;; GOOD
(define-public (do-something-cool (userId uint))
    (let
        (
            (firstVar (+ 1 10))
            (secondVar (pow u2 u8))
        )
        (ok (call-function firstVar secondVar))
    )
)

[314159265359879]

@LNow Thank you for kicking this off, excellent start. Friedger also voiced the idea for standard error codes should that be part of this SIP or separate? To me it seems reasonable to make it part of this more general idea for a clarity coding standard.

[LNow]

If he was talking about error codes returned by native function, I think they need to be and are described in language reference. Yet, I see nothing against listing them in codding standards as well.

[jcnelson]

I think this is a good idea! While the blockchain itself can't enforce a coding standard, a SIP that describes a coding standard could be used to inform the implementation of developer tooling.

[obycode]

I like this idea too. Thanks @LNow! I'd happily add a linter pass to Clarinet once the styles are agreed upon.

[whoabuddy]

We used this standard to help make the CityCoins contracts much more readable and almost one year later it was totally worth it.

The fact you can look the code and quickly distinguish between functions, variables, maps, and constants is very useful especially as the complexity of the contract increases.

I define my TypeScript interfaces/types with PascalCase, which fits perfectly with defining the data structure within a map, e.g.

https://github.com/citycoins/api/blob/62d2f669902d588d95eac3956557bd1dd6b5bbe7/src/types/mining.ts#L2-L15

Properly laying out the parentheses is extremely helpful too. If someone is coming into Clarity with no experience with a LISP-like language, it helps with getting used to the nuances of how the code is structured, e.g. using let:

(define-public (double-foo (foo uint))
  (let
    (
      ;; define let variables here
      (bar (* foo u2))
    )
    ;; operations / return
    (ok bar)
  )
)

It also helps with realizing where you can simplify things, e.g. the function above could avoid the let statement completely:

(define-public (double-foo (foo uint))
  (ok (* foo u2))
)

One more thing to note - if voted in, this format would be extremely helpful for contract documentation as well, especially the page for function references, e.g.

(let ((a 2) (b (+ 5 6 7))) (print a) (print b) (+ a b)) ;; Returns 20
(let ((a 5) (c (+ a 1)) (d (+ c 1)) (b (+ a c d))) (print a) (print b) (+ a b)) ;; Returns 23

https://docs.stacks.co/references/language-functions#let

[whoabuddy]

@LNow any thoughts on the error format here? Recently we discussed doing it one of two ways:

Current:

(define-constant ERR_UNAUTHORIZED u401)

(define-public (some-function)
  (asserts! (is-eq tx-sender contract-caller)
    (err ERR_UNAUTHORIZED)
  )
)

Updated method:

(define-constant ERR_UNAUTHORIZED (err u401))

(define-public (some-function)
  (asserts! (is-eq tx-sender contract-caller)
    (ERR_UNAUTHORIZED)
  )
)

[LNow]

Most of the time I prefer updated version.

[obycode]

What's the status on this issue? Why did it get closed? @LNow

[LNow]

@obycode I closed this and few other issues because they were obsolete and I've lost all interest to do anything but close them.

[Hero-Gamer]

Is this a great improvement that is beneficial to the all of Stacks? If so, I'd really love to see it revived somehow.