Skip to content

Commit

Permalink
Linter: Automate the generation of documentation for ink-docs (#1972)
Browse files Browse the repository at this point in the history
* feat(linter): Automate the generation of documentation for ink-docs

This commit introduces a script that extracts documentation from
`ink_linting` lints and saves it to the local [ink-docs](https://github.com/paritytech/ink-docs) directory.

The markdown formatting of the lints was also improved.

* chore(script): Update after #2032 is merged

* chore(linter): Wrap in docstring to improve appearance

* Update scripts/generate_linter_docs.sh

---------

Co-authored-by: Michael Müller <mich@elmueller.net>
  • Loading branch information
jubnzv and cmichi authored Jan 18, 2024
1 parent 0ada33b commit b05e7f2
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 29 deletions.
35 changes: 19 additions & 16 deletions linting/extra/src/primitive_topic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,42 +54,45 @@ use rustc_session::{
};

declare_lint! {
/// **What it does:** Checks for ink! contracts that use
/// the [`#[ink(topic)]`](https://use.ink/macros-attributes/topic) annotation with primitive
/// number types. Topics are discrete events for which it makes sense to filter. Typical
/// examples of fields that should be filtered are `AccountId`, `bool` or `enum` variants.
/// ## What it does
/// Checks for ink! contracts that use the
/// [`#[ink(topic)]`](https://use.ink/macros-attributes/topic) annotation with primitive number
/// types. Topics are discrete events for which it makes sense to filter. Typical examples of
/// fields that should be filtered are `AccountId`, `bool` or `enum` variants.
///
/// **Why is this bad?** It typically doesn't make sense to annotate types like `u32` or `i32`
/// as a topic, if those fields can take continuous values that could be anywhere between
/// `::MIN` and `::MAX`. An example of a case where it doesn't make sense at all to have a
/// topic on the storage field is something like `value: Balance` in the examle below.
///
/// **Example:**
/// ## Why is this bad?
/// It typically doesn't make sense to annotate types like `u32` or `i32` as a topic, if those
/// fields can take continuous values that could be anywhere between `::MIN` and `::MAX`. An
/// example of a case where it doesn't make sense at all to have a topic on the storage field
/// is something like `value: Balance` in the examle below.
///
/// ## Example
/// ```rust
/// // Good
/// // Filtering transactions based on source and destination addresses.
/// // Bad
/// // It typically makes no sense to filter `Balance`, since its value may varies from `::MAX`
/// // to `::MIN`.
/// #[ink(event)]
/// pub struct Transaction {
/// #[ink(topic)]
/// src: Option<AccountId>,
/// #[ink(topic)]
/// dst: Option<AccountId>,
/// #[ink(topic)]
/// value: Balance,
/// }
/// ```
///
/// Use instead:
///
/// ```rust
/// // Bad
/// // It typically makes no sense to filter `Balance`, since its value may varies from `::MAX`
/// // to `::MIN`.
/// // Good
/// // Filtering transactions based on source and destination addresses.
/// #[ink(event)]
/// pub struct Transaction {
/// #[ink(topic)]
/// src: Option<AccountId>,
/// #[ink(topic)]
/// dst: Option<AccountId>,
/// #[ink(topic)]
/// value: Balance,
/// }
/// ```
Expand Down
7 changes: 3 additions & 4 deletions linting/extra/src/storage_never_freed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,17 @@ use std::collections::{
};

declare_lint! {
/// **What it does:**
/// ## What it does
/// This lint ensures that for every storage field with a collection type, when there is an
/// operation to insert new elements, there's also an operation for removing elements.
///
/// **Why is this bad?**
/// ## Why is this bad?
/// When a user executes a contract function that writes to storage, they have to put a
/// deposit down for the amount of storage space used. Whoever frees up that storage at some
/// later point gets the deposit back. Therefore, it is always a good idea to make it possible
/// for users to free up their storage space.
///
/// **Example:**
///
/// ## Example
/// In the following example there is a storage field with the `Mapping` type that has an
/// function that inserts new elements:
///
Expand Down
21 changes: 13 additions & 8 deletions linting/extra/src/strict_balance_equality.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,22 +72,25 @@ use std::collections::{
};

declare_lint! {
/// **What it does:** Looks for strict equalities with balance in ink! contracts.
/// ## What it does
/// Looks for strict equalities with balance in ink! contracts.
///
/// **Why is this bad?** The problem with strict balance equality is that it is always possible
/// to forcibly send tokens to a contract. For example, using
/// ## Why is this bad?
/// The problem with strict balance equality is that it is always possible to forcibly send
/// tokens to a contract. For example, using
/// [`terminate_contract`](https://paritytech.github.io/ink/ink_env/fn.terminate_contract.html).
/// In such a case, the condition involving the contract balance will work incorrectly, what
/// may lead to security issues, including DoS attacks and draining contract's gas.
///
/// **Known problems**: There are many ways to implement balance comparison in ink! contracts.
/// This lint is not trying to be exhaustive. Instead, it addresses the most common cases that
/// may occur in real-world contracts and focuses on precision and lack of false positives.
///
/// **Example:**
/// ## Known problems
/// There are many ways to implement balance comparison in ink! contracts. This lint is not
/// trying to be exhaustive. Instead, it addresses the most common cases that may occur in
/// real-world contracts and focuses on precision and lack of false positives.
///
/// ## Example
/// Assume, there is an attacker contract that sends all its funds to the target contract when
/// terminated:
///
/// ```rust
/// #[ink::contract]
/// pub mod attacker {
Expand All @@ -101,6 +104,7 @@ declare_lint! {
///
/// If the target contains a condition with strict balance equality, this may be manipulated by
/// the attacker:
///
/// ```rust
/// #[ink::contract]
/// pub mod target {
Expand All @@ -116,6 +120,7 @@ declare_lint! {
///
/// This could be mitigated using non-strict equality operators in the condition with the
/// balance:
///
/// ```rust
/// #[ink::contract]
/// pub mod target {
Expand Down
3 changes: 2 additions & 1 deletion linting/mandatory/src/no_main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ declare_lint! {
/// ## Example
///
/// ```rust
/// // Bad: Contract does not contain the `no_main` attribute, so it cannot be compiled to Wasm
/// // Bad: Contract does not contain the `no_main` attribute,
/// // so it cannot be compiled to Wasm
/// #![cfg_attr(not(feature = "std"), no_std)]
/// #[ink::contract]
/// mod my_contract { /* ... */ }
Expand Down
57 changes: 57 additions & 0 deletions scripts/generate_linter_docs.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#!/usr/bin/env bash

set -eu

usage() {
echo "Usage: $0 PATH"
echo "Creates or updates the documentation for ink_linting hosted at use.ink."
echo
echo "Arguments:"
echo " PATH Path to the local clone of https://github.com/paritytech/ink-docs"
echo
echo "Examples:"
echo " $0 ~/ink-docs"
}

if [ ! $# -eq 1 ]; then
usage
exit 1
fi

ink_docs_dir="$1"
tmp_file=$(mktemp /tmp/linting_docs.XXXXXX.md)
had_update=0
for lint_src in linting/{mandatory,extra}/src/*.rs; do
lint_name="$(basename "${lint_src%.*}")"
lint_doc_file="$ink_docs_dir/docs/linter/rules/$lint_name.md"
lint_doc_filestring="$(sed -n '/^declare_lint! {$/,/^}$/p' "$lint_src")"
[[ -z "$lint_doc_filestring" ]] && continue

# Save the extracted documentation to the temporary file
truncate -s 0 "$tmp_file"
{
echo "---"
echo "title: $lint_name"
echo "hide_title: true"
echo "description: $lint_name lint documentation"
echo "---"
echo "# $lint_name"
printf "%s" "$lint_doc_filestring" | sed -n 's/^\s*\/\/\/\s\?\(.*\)$/\1/;T;p'
} >> "$tmp_file"

if [[ ! -f "$lint_doc_file" ]]; then
echo "$lint_name: created documentation"
had_update=1
mv "$tmp_file" "$lint_doc_file"
continue
fi

if cmp -s "$tmp_file" "$lint_doc_file"; then
echo "$lint_name: no changes"
else
echo "$lint_name: updated"
mv "$tmp_file" "$lint_doc_file"
fi
done

[[ $had_update -eq 1 ]] && echo -e "\nPlease add created documentation sections to $ink_docs_dir/sidebars.js"

0 comments on commit b05e7f2

Please sign in to comment.