<!-- vi: tw=119
-->

# Contributing guide for vuln_demo

## Project installation

This project uses [`poetry`](https://python-poetry.org/docs/). It's a tool for managing python virtual environments. If
you haven't heard of those, they're essentially a mini installation of python used purely for the project you're working
on (as opposed to using a single global python installation for everything, which is prone to conflicts, as different
projects might need different versions of the same package). Follow the linked documentation for installation
instructions.

Once installed, you will want to create a new environment for mcproto, with all of it's dependencies installed. To do
that, enter the clonned repository in your terminal, and run:

```bash
poetry install
```

Note that you will want to re-run this command each time our dependencies are updated, to stay in sync with the project.

After that, the environment will contain all of the dependencies, including various executable programs, such as
`pyright`. One of these executable programs is also `python`, which is the python interpreter for this environment,
capable of interacting with all of the installed libraries.

You will now need to make your terminal use the programs from this environment, rather than any global versions that you
may have installed, so that you can use the tools in it when working on the project. Some IDEs/editors are capable of
doing this for you automatically when you open the project. If yours isn't, you can run:

```bash
poetry shell
```

You can then start your IDE from the terminal, after you ran this command, and it should pick up the python environment
created by poetry.

You can also just prefix any command with `poetry run` (e.g. `poetry run python`) to use the executable from the
environment, without activating it, however you will almost always want to activate the environment instead.

For more info about poetry, make sure to check their amazing official documentation: `https://python-poetry.org/docs/`,
these include installation instructions, explain how to add new dependencies to the project, or how to remove some, and
everything else you'd need to know.

## Style Guide

For clarity and readability, adhering to a consistent code style across the whole project is very important. It is not
unusual that style adjustments will be requested in pull requests.

It is always a good practice to review the style of the existing code-base before adding something new, and adhere to
that established style. That applies even if it isn't the style you generally prefer, however if you think a code style
change of some kind would be justified, feel free to open an issue about it and tell us what exactly should be changed,
and why you think this change is important. If you want to, you can also ask to be assigned to the issue and work on
changing the style in the code-base in your own PR. (Hey you may even get to edit this section!)

> A style guide is about consistency. Consistency with this style guide is important. Consistency within a project is
> more important. Consistency within one module or function is the most important.
>
> However, know when to be inconsistent -- sometimes style guide recommendations just aren't applicable. When in doubt,
> use your best judgment. Look at other examples and decide what looks best. And don't hesitate to ask!
> — [PEP 8, the general Style Guide for Python Code](https://peps.python.org/pep-0008/)

### Automatic linting

As there is a lot of various styling rules we adhere to in our code base, and obviously, describing all of them in a
style guide here would just take way too long, and it would be impossible to remember anyway. For that reason, we use
automated tools to help us catch any style violation without manual review!

The primary tool that we use in this project is [`ruff`](https://beta.ruff.rs/docs/). It is a general python linter,
formatter and import sorter.

You can read more about it in the section below. It is important that you familiarize yourself with it and it's
standalone usage, but it would of course be very annoying to have to execute the commands to run ruff manually, so
while there will be instructions on how to do that, you should pretty much always prefer direct IDE/editor integration,
which is mentioned [here](#editor-integration), and make use of [pre-commit](#pre-commit).

#### Ruff linter & Formatter

Ruff is an all-in-one linter & formatter solution, which aims to replace the previously very popular
[`flake8`](https://flake8.pycqa.org/en/latest/) linter, [`isort`](https://pycqa.github.io/isort/) import sorter and
[`black`](https://black.readthedocs.io/en/stable/) formatter. Ruff is faster (written in rust! 🦀) and includes most of
the popular flake8 extensions directly. It is almost 1:1 compatible with black, which means the way it formats code is
pretty much the same, with only some very subtle differences.

You can check the ruff configuration we're using in [`pyproject.toml`](./pyproject.toml) file, under the `[tool.ruff]`
category (and it's subcategories), you can find the enabled linter rules there, and some more specific configuration,
like line length, python version, individual ignored lint rules, and ignored files.

To run `ruff` **linter** on the code, you can use `ruff check .` command, while in the project's root directory (from
an activated poetry environment, alternatively `poetry run ruff .`). Ruff also supports some automatic fixes to many
violations it founds, to enable fixing, you can use `ruff check --fix`. This will also run the `isort` integration.

If you find a rule violation in your code somewhere, and you don't understand what that rule's purpose is, `ruff` evens
supports running `ruff rule [rule id]` (for example `ruff rule ANN401`). These explanations are in markdown, so I'd
recommend using a markdown renderer such as [`glow`](https://github.com/charmbracelet/glow) (on Arch linux, you can
install it with: `pacman -S glow`) and piping the output into it for a much nicer reading experience: `ruff rule ANN401
| glow`.

To run `ruff` **formatter** on the code, you can simply execute `ruff format .` command (also needs an activated poetry
environment). This will automatically format all files in the code-base.

### Docstring formatting directive

The `ruff` linter uses various rules from the `flake8-docstrings` rule-set to enforce a specific standardized docstring
formatting. However, these rules will only enforce the basic structure of docstrings, and where they need to be
specified. In addition to these rules, it is important to mention the rules on the style of the content in the
docstrings themselves.

Specifically, we follow Sphinx, and the docstrings should be written in the restructuredtext format. Sphinx supports
various directives that allow specifying notes, describe how to add citations, references to other
functions/documentation, how to add tables and a bunch of other things. You can read up on these guidelines in the
official Sphinx documentation: [here](https://www.sphinx-doc.org/en/master/usage/restructuredtext/)

The use of this style is important, as many editors recognize it, and can show you properly formatted descriptions on
functions or other types of objects upon hovering on them. This format is also useful for automatic generation of
documentation using Sphinx, as it's much easier and more comfortable to simply add the descriptions for each function
directly in the code, than having to replicate it manually in the standalone source code for the project's
documentation, and keep it up to date as new changes to the code are introduced.

Below is a quick example that demonstrates some of how this code style can look. This showcases both the general style
of our docstrings, and the use of restructuredtext in them. However, it is heavily recommended that you read up on this
in the linked documentation, to know what markup is available and how to use it. You can also see a bunch of examples
from the existing code in this code-base, if you just want a better quick glance.

```python
def donut(a: bool, b: str) -> None:
    """Short one-line description of the function."""


def pineapple(a: bool, b: str) -> str:
    """One-line description of the function telling us what it's about.

    Detailed multiline description.
    This may include the full explanation of how this function should be used.

    We can also have multiple sections like this.
    For example to include further use instruction with some examples or perhaps
    with an explanation of how the function works, if it's relevant.
    """


def add(x: int, y: int) -> int:
    """Add two numbers together.

    :param x: Number 1 to be added.
    :param y: Number 2 to be added.
    :returns: Result of ``x + y`` addition.
    """

def basic_rest(a: bool, b: str) -> None:
    """My funtcion teaches you about some basic reST formatting.

    Some types of text formatting in restructuredtext (reST) are very similar to
    those supported in markdown, for example, **this text will be bold**, *This
    text will be emphasised (italic)*.

    One important difference between reST and markdown is the use of double backquotes
    to include inline code (literals), instead of just a single backquote: ``code``.

    * This is a bulleted list.
    * It has three items, the second one has a nested list.
      * This is a nested list.
      * It has two items.
    * This is the third item, continuing the parent list.

    1. This is a numbered list
    2. It has two items.

    #. This is a numbered list
    #. It has two items too.

    .. versionadded:: 2.5
    .. deprecated:: 3.1
       Use :func:`coconut` instead.
    .. versionremoved:: 4.0
       The :func:`coconut` is more flexible, and should be used instead.
       This function will be removed in an upcomming major release.
    .. note::
        This is a note directive, it will show up nicely formatted in the
        documentation.
    .. warning::
        This is a warning directive. It may contain some important info about
        this function, and should be used instead of the note directive, if you're
        describing some information regarding security.
    """

def hyperlinks(q: float) -> int:
    """This function teaches you about hyperlinks

    Restructured text also supports hyperlinks. For named links, you can use:
    `Link text <https://domain.invalid>`_. For inline links, that just show the URL,
    you can simply use: `<https://domain.invalid>`_.

    You can also separate the link and target definition, like this: `my link`_.

    .. _my link: https://domain.invalid

    .. deprecated: 2.5
    """

def reference(param: str) -> None:
    """This teaches you about references.

    You will often need to refer to external (or internal) functions, classes or
    other things in your docstrings. To do so, you can follow this guide:

    * To refer to the parameter/argument that this function takes, simply use ``param``.
    * To refer to another class, use :class:`MyClass`.
    * To refer to another function, use :func:`my_func`.
    * To refer to another method, in the class this method is in, use :meth:`my_method`.
    * To refer to an attribute of the class this method is in, use :attr:`my_attr`
    * To refer to a constant, use :const:`FOOBAR`.
    * To refer to an exception, use :exc:`FoobarError`.
    * To refer to an object, use :obj:`my_object`.

    If you need to refer to objects defined outside of this file, you can use the
    fully qualified path to them, like: :class:`my_module.foo.bar.MyClass`. This
    will show to full path in the documentation too though, if you don't want that,
    you can also use: :class:`~my_module.foo.bar.MyClass`, which will only show up
    as ``MyClass`` in the final docs.

    You can also refer to entire modules: :mod:`itertools`. Since ``itertools`` is
    actually a part of the standard library, sphinx can even produce links that go
    back to Python's official docs for the ``itertools`` modules.

    You can even refer to PEPs, like the :PEP:`287`.

    .. seealso::
        This directive can be used to refer to some other documentation or external
        documents.

        It can be useful to put your references in, instead of just having them in
        the docstring's text directly.
    """
```

Another general rule of thumb when writing docstrings is to generally stick to using an imperative mood.

Imperative mood is a certain grammatical form of writing that expresses a clear command to do something.

**Use:** "Build a player object."
**Don't use:** "Returns a player object."

Present tense defines that the work being done is now, in the present, rather than in the past or future.

**Use:** "Build a player object."
**Don't use:** "Built a player object." or "Will build a player object."

## Type hinting

[PEP 484](https://www.python.org/dev/peps/pep-0484/) formally specifies type hints for Python. You can specify type
hints for a function, in addition to just parameter names, allowing you to quickly understand what kind of parameter
this is. Most IDEs/editors will even be able to recognize these type hints, and provide auto-completion based on them.
For example, if you type hint a parameter as `list`, an editor can suggest list methods like `join` or `append`. Many
editors will even show you the type hint on the argument in the function's signature, when you're trying to call it,
along with the parameter name making it really easy to understand what you're supposed to pass without even looking at
the docstring.

For example, an untyped function can look like this:

```python
def divide(a, b):
    """Divide the two given arguments."""
    return a / b
```

With type-annotations, the function looks like this:

```python
def divide(a: int, b: int) -> float:
    """Divide the two given arguments."""
    return a / b
```

Thankfully python type-hinting is fairly easy to understand, but if you do want to see some rather interesting
resources for a bit more advanced concepts such as type variables or some more complex types like `typing.Callable`,
we've compiled a quick list of really amazing resources about these type hinting practice.

- Python documentation from `typing` library: <https://docs.python.org/3/library/typing.html>
- MyPy documentation (very extensive but quite beginner friendly): <https://mypy.readthedocs.io/en/stable/>
- Decorator Factory blog about typing: <https://decorator-factory.github.io/typing-tips/>
- Typing Generics (advanced): <https://itsdrike.com/posts/typing-generics/>

### Enforcing type hints - Type checker

Even though the type hints can be pretty useful in knowing what the function variables are expected to be and they also
provide better auto-completion, if we're not careful, we could soon end up violating our own type specifications,
because by default python doesn't enforce these type-hints in any way. To python, they're not much more than comments.

To make sure that our code-base really is correct type-wise, we use a tool that looks at the code statically (similarly
to a linter), and analyzes the types, finding any inconsistencies. Using a type-checker can be very beneficial,
especially to bigger projects, as it can quickly catch mistakes we made based on purely the types, without even having
to run the code. So many times, you'll see issues before actually testing things out (with unit-tests, or manually). In
a lot of cases, type checkers can even uncover many things that our unit tests wouldn't find.

There are many python type-checkers available, the most notable ones being `mypy` and `pyright`. We decided to use
`pyright`, because it has great support for many newer typing features. Pyright can be used from the terminal as a
stand-alone linter-like checker, by simply running `pyright .` (from within an activated virtual environment). But just
like with linters, you should ideally just [include it into your editor directly](#editor-integration). We also run
pyright automatically, as a part of [pre-commit](#pre-commit).

## Pre-commit

Now that you've seen the linters, formatters, type-checkers and other tools that we use in the project, you might be
wondering whether you're really expected to run all of those commands manually, after each change. And of course, no,
you're not, that would be really annoying, and you'd probably also often just forget to do that.

So, instead of that, we use a tool called `pre-commit`, which creates a [git
hook](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks), that will automatically run before each commit you
make. That means each time when you make a commit, all of these tools will run over the code you updated, and if any of
these linters detects an issue, the commit will be aborted, and you will see which linter failed, and it's output
telling you why.

To install pre-commit as a git hook all you need to do is to run `pre-commit install` from an activated poetry
environment, installing it into the git repository as a hook running before every commit. That said, you can also run
pre-commit without having to install it (or without having to make a commit, even if installed). To do that, simply
execute: `pre-commit run --all-files`. Note that the installed hook will only run the linters on the files that were
updated in the commit, while using the command directly will run it on the whole project.

You can find pre-commit's configuration the [`.pre-commit-config.yaml`](./.pre-commit-config.yaml) file, where we
define which tools should be ran and how. Currently, pre-commit runs ruff linter, ruff formatter, pyright and also a
checker for some issues in TOML/YAML files.

Even though in most cases enforcing linting before each commit is what we want, there are some situations where we need
to commit some code which doesn't pass these checks. This can happen for example after a merge, or as a result of
making a single purpose small commit without yet worrying about linters. In these cases, you can use the `--no-verify`
flag when making a commit, telling git to skip all of the pre-commit hooks and commit normally. You can also only skip
a specific hook(s), by setting `SKIP` environmental variable (e.g. `SKIP=pyright`, or
`SKIP=ruff-linter,ruff-formatter`), the names of the individual hooks are their ids, you can find those in the
configuration file for pre-commit.

However, this kind of verification skipping should be used sparingly. We value a clean history which consistently
follows our linting guidelines, and making commits with linting issues only leads to more commits, fixing those issues
later. If you really do need to skip the linters though, you should then wait until you create another commit fixing
the issues before pushing the code to github, to avoid needlessly failing the automated workflows, which run pre-commit
themselves (amongst other things).

## Editor Integration

Even with pre-commit, it would still be very annoying to have to only run the linters during the commit, because with
the amount of rules we have, and especially if you're not used to following many of them, you will make a lot of
mistakes. Because of that, we heavily recommend that you integrate these tools into your IDE/editor directly. Most
editors will support integration will all of these tools, so you shouldn't have any trouble doing this.

If you're using neovim, I would recommend setting up LSP (Language Server Protocol), and installing Pyright, as it has
language server support built into it. Same thing goes with `ruff`, which has an LSP implementation
[`ruff-lsp`](https://github.com/astral-sh/ruff-lsp).

On vscode, you can simply install the following extensions:

- [pylance (pyright)](https://marketplace.visualstudio.com/items?itemName=ms-python.vscode-pylance)
- [ruff](https://marketplace.visualstudio.com/items?itemName=charliermarsh.ruff)

Note that with Pylance, you will also need to enable the type checking mode, by setting
`"python.analysis.typeCheckingMode": "basic"` in `settings.json`. You can use `.vscode/settings.json` for per-project
settings, to only enable type-checking for this project, or enable it globally in your user settings).

## Making Great Commits

A well-structured git log is key to a project's maintainability; it provides insight into when and why things were done
for future maintainers of the project.

Commits should be as narrow in scope as possible. Commits that span hundreds of lines across multiple unrelated
functions and/or files are very hard for maintainers to follow. After about a week, they'll probably be hard for you to
follow too.

Please also avoid making a lot minor commits for fixing test failures or linting errors. Instead, run the linters before
you push, ideally with [pre-commit](#pre-commit).

We've compiled a few resources on making good commits:

- <https://itsdrike.com/posts/great-commits/>
- <https://chris.beams.io/posts/git-commit/>
- <https://dhwthompson.com/2019/my-favourite-git-commit>
- <https://thoughtbot.com/blog/5-useful-tips-for-a-better-commit-message>
- <https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html>

## Footnotes

This could be a lot to remember at once, but you can use this document as a resource while writing the code for our
repository and cross-check that your styling is following our guidelines and that you're practicing the rules that
we've set here.

This document was inspired by:

- [Python Discord's CONTRIBUTING agreement.](https://github.com/python-discord/bot/blob/master/CONTRIBUTING.md)
- [mcproto CONTRIBUTING.md file](https://github.com/py-mine/mcproto/blob/main/CONTRIBUTING.md)
