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

WIP: Cadence Style Guide #3

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

alilloig
Copy link
Member

Closes #2

Description

Developers need better guidelines for how to write clean cadence code. Define those guidelines at docs/cadence-style-guide.md. Early PR containing only table of contents to seek consensus about which points should the guide contain and check everyone's thoughts on conflictive aspects.


  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@alilloig alilloig added the SC-Eng Issues that we want to see surfaced in SC-Eng ZH Board label Jul 28, 2022
@alilloig alilloig linked an issue Jul 28, 2022 that may be closed by this pull request
@alilloig alilloig changed the title Cadence Style Guide WIP: Cadence Style Guide Jul 28, 2022
Copy link
Member

@joshuahannan joshuahannan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far!

.getCapability(ContractName.SomeAbsurdlyLongForSomeReasonPathName)
.borrow<&SomeResource{ContractName.InterfaceName}>()
```
Keep allways those concatenated function calls idented from the object they are being called from.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Keep allways those concatenated function calls idented from the object they are being called from.
Always keep those concatenated function calls indented from the object they are being called from.

cadence-style-guide.md Outdated Show resolved Hide resolved
cadence-style-guide.md Outdated Show resolved Hide resolved
cadence-style-guide.md Outdated Show resolved Hide resolved
cadence-style-guide.md Show resolved Hide resolved
cadence-style-guide.md Outdated Show resolved Hide resolved
## Comments
Comments are a vital part of smart contracts, allowing users to easily understand what they are getting involve with.
### High level documentation at the begining of files (contracts, transactions and scripts)
Top Level comments and comments for types, fields, events, and functions should use `///` (three slashes) because there is a cadence docs generating tool that picks up three slash comments to auto-generate docs.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you link to the docgen section in the cadence repo here?

Comment on lines 37 to 40
Functions should be commented with a:
* Description
* Parameter descriptions (unless it is obvious)
* Return value descriptions (unless it is obvious)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be removed from here and created as an issue in this repo so we can discuss it

cadence-style-guide.md Outdated Show resolved Hide resolved
cadence-style-guide.md Outdated Show resolved Hide resolved
alilloig and others added 2 commits July 28, 2022 19:47
Co-authored-by: Joshua Hannan <joshua.hannan@dapperlabs.com>
Use 4 spaces per indentation level.
Prefer spaces over tabs as indentation method, mixing them should be avoided.
## Line length
A line length of 80 characters is recommended for Cadence. This could be particularly challenging when writing capability-related lines, for instance:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree the guide should recommend a reasonable maximum line length, but we're not writing code on punch cards anymore. Maybe something like 100?

Comment on lines +10 to +13
1. [Naming and capitalization](#naming-and-capitalization)
## Indentation
Use 4 spaces per indentation level.
Prefer spaces over tabs as indentation method, mixing them should be avoided.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add line breaks between Markdown elements, to make it easier to read and edit:

Suggested change
1. [Naming and capitalization](#naming-and-capitalization)
## Indentation
Use 4 spaces per indentation level.
Prefer spaces over tabs as indentation method, mixing them should be avoided.
1. [Naming and capitalization](#naming-and-capitalization)
## Indentation
Use 4 spaces per indentation level.
Prefer spaces over tabs as indentation method, mixing them should be avoided.

```cadence
letResourceRef = account.getCapability(ContractName.SomePathName).borrow<&SomeResource{ContractName.InterfaceName}>()
```
This generic line of code that can be found in many transactions that need to borrow a reference to a certain object from a capability is 118 characters long by its own without any indentation. Keeping this line readable should be an objective of any Cadence developer. There are some simple patterns that could be observed in order to accomplish this:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add line breaks after logical ends, i.e. put logical parts of the paragraph on separate lines:

Suggested change
This generic line of code that can be found in many transactions that need to borrow a reference to a certain object from a capability is 118 characters long by its own without any indentation. Keeping this line readable should be an objective of any Cadence developer. There are some simple patterns that could be observed in order to accomplish this:
The following line of code is an example of code that can be found in many transactions.
It borrows a reference to a certain object from a capability, and is 118 characters long without any indentation.
Keeping this line readable should be an objective of any Cadence developer.
There are some simple practices that should be followed in order to accomplish this:

letResourceRef = account.getCapability(ContractName.SomePathName).borrow<&SomeResource{ContractName.InterfaceName}>()
```
This generic line of code that can be found in many transactions that need to borrow a reference to a certain object from a capability is 118 characters long by its own without any indentation. Keeping this line readable should be an objective of any Cadence developer. There are some simple patterns that could be observed in order to accomplish this:
+ Prefer to always break the line before the `borrow` call.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+ is uncommen in Markdown lists, prefer to use * or -

This generic line of code that can be found in many transactions that need to borrow a reference to a certain object from a capability is 118 characters long by its own without any indentation. Keeping this line readable should be an objective of any Cadence developer. There are some simple patterns that could be observed in order to accomplish this:
+ Prefer to always break the line before the `borrow` call.
```cadence
letResourceRef = account.getCapability(ContractName.SomePathName)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
letResourceRef = account.getCapability(ContractName.SomePathName)
let resourceRef = account.getCapability(ContractName.SomePathName)

```
+ If indentation or a long path name makes the "getCapability" line go further than 80 characters an additional line break can be included
```cadence
letResourceRef = account
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
letResourceRef = account
let resourceRef = account

## Comments
Comments are a vital part of smart contracts, allowing users to easily understand what they are getting involved with.
### High level documentation at the begining of files (contracts, transactions and scripts)
Top Level comments and comments for types, fields, events, and functions should use `///` (three slashes) because [the cadence docs generating tool](https://github.com/onflow/cadence/tree/master/tools/docgen) picks up three slash comments to auto-generate docs.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Top Level comments and comments for types, fields, events, and functions should use `///` (three slashes) because [the cadence docs generating tool](https://github.com/onflow/cadence/tree/master/tools/docgen) picks up three slash comments to auto-generate docs.
Top-level comments and comments for types, fields, events, and functions should use `///` (three slashes) because [the Cadence documentation generator](https://github.com/onflow/cadence/tree/master/tools/docgen) uses them to differentiate between normal comments and documentation comments.

Comment on lines +42 to +43
### Contracts
Contract and contract interfaces names should always follow PascalCase, for instance:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Contracts
Contract and contract interfaces names should always follow PascalCase, for instance:
### Types
Type names should always follow CamelCase and start with an uppercase letter, for instance:

```cadence
pub contract ExampleToken
```
### Fields
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Fields
### Fields
Field names should always follow camelCase, and start with a lowercase letter, for instance:

```
// Different rule for regular fields than for path fields?
### Functions
Function names should always follow camelCase, for instance:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Function names should always follow camelCase, for instance:
Function names should always follow camelCase, and start with a lowercase letter, for instance:

pub var totalSupply
```
```cadence
pub var VaultStoragePath
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub var VaultStoragePath
pub let VaultStoragePath

Copy link

@bluesign bluesign Sep 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think rule here is related to being constant, but I am not sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SC-Eng Issues that we want to see surfaced in SC-Eng ZH Board
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Cadence Style Guide
4 participants