Skip to content

CONTRIBUTING.md

Latest commit

 

History

History
247 lines (169 loc) · 9.65 KB

CONTRIBUTING.md

File metadata and controls

247 lines (169 loc) · 9.65 KB

Contributing Guide

Style

We keep code clean and constantly refactor it for better readability, maintainability, and performance.

  • It is totally fine and encouraged to refactor existing code and send PRs for that, even if it is a small renaming or a typo fix.
  • Do not mix up refactoring with new features/bug fixes in a single PR.

AI Generated Code

Of course you can use AI tools to help you write code (who can ban you from that?) but in this case, please make sure to:

  • Manually review and test all AI-generated code before submitting it
  • State clearly in the PR description which parts of the code were generated by AI tools
  • Prefer small, focused AI-generated snippets over large chunks of code
  • Ensure that AI-generated code adheres to the project's coding style and guidelines

Warning

If we find AI-generated code that is not reviewed or tested, we may ask you to remove it or even close the PR.

General

Dependencies

We prefer to keep dependencies to a minimum so we can control the code quality, build process, and security. Also, we know how slow a compilation can get with too many dependencies. Therefore, if you are thinking about adding a new dependency, please consider checking with the maintainers first.

Branching Strategy

We use the main branch as the default branch and create tags for releases. For new features and bug fixes, please create feature branches from main and open pull requests against main.

Before starting work on a PR, please ensure that there is an issue created for it and that it is assigned to you. If your planned work is only a subset of an existing issue, create a new issue for your specific task and link it to the parent one.

Branch Naming

Make branch names inherit the issue that it addresses. The pattern is: <issue-number>-<type>-<short-description>.

Examples:

  • If you work on a new feature "9 - Develop Linker", name your branch 9-feature-develop-linker.
  • If you work on a bug fix "15 - Fix Parser Crash", name your branch 15-bug-fix-parser-crash.

Commit Messages

Please, do not use emoji in commit messages. Keep them simple and descriptive. Avoid tagging people @ because in case of rebasing or playing with history it may lead to unwanted notifications.

Clippy

We use clippy, but sometimes it is too noisy. In those cases, it is OK to disable specific warnings.

must_use

When applicable, use the #[must_use=reason] version of the must_use attribute.

Code

Tests

We tend to have as good test coverage as possible. At the same time we keep a balance between writing redundant tests that can make the codebase less readable and maintainable. Please keep in mind this balance when writing tests.

Every PR (except for documentation-only changes and renaming) must include tests that cover the changes made.

Unit Tests

When writing unit tests, you probably need to have Inference source code as input. In this case, locate the code on top of the test in a compact format. For example:

#[test]
fn test_parse_simple_function() {
    let source = r#"fn add(a: i32, b: i32) -> i32 { return a + b; }"#;
    let ast = build_ast(source.to_string());
    let source_files = &ast.source_files;
    assert_eq!(source_files.len(), 1);

    let definitions = &source_files[0].definitions;
    assert_eq!(definitions.len(), 1);
}

As you can see, the source code is compact and fits within one line. When it makes sense, compact the code because spaces are ignored by the parser anyway.

Code Generation Tests

Code generation (codegen) tests validate that the actual lowering matches the expected one. With the help of some utilities, you can write such tests easily. Directories tests/src/codegen and tests/test_data/codegen are paired and help to organize codegen tests.

Modules in the codegen directory must reflect the structure of the test_data/codegen directory. For example, if you have a test codegen::wasm::base::trivial_test then the corresponding source file must be located at tests/test_data/codegen/wasm/base/trivial/trivial.inf and an expected binary at tests/test_data/codegen/wasm/base/trivial/trivial.wasm. Each test case lives in its own subdirectory named after the test. And if it is, then you can write a test as easy as:

    #[test]
    fn trivial_test() {
        let test_name = "trivial";
        let test_file_path = get_test_file_path(module_path!(), test_name);
        let source_code = std::fs::read_to_string(&test_file_path)
            .unwrap_or_else(|_| panic!("Failed to read test file: {test_file_path:?}"));
        let actual = wasm_codegen(&source_code);
        let expected = get_test_wasm_path(module_path!(), test_name);
        let expected = std::fs::read(&expected)
            .unwrap_or_else(|_| panic!("Failed to read expected wasm file for test: {test_name}"));
        assert_wasms_modules_equivalence(&expected, &actual);
    }

Of course, before setting up such a test, it is absolutely required to check binaries and provide an execution test suite for them. After review, execution tests can be removed. For running wasm binaries, you can use the wasmtime CLI tool.

#[should_panic]

Do not use #[should_panic] tests. Instead, explicitly check for None, Err, etc.

Rationale: #[should_panic] is a tool for library authors to make sure that the API does not fail silently when misused. infc is not a library, we don't need to test for API misuse, and we have to handle any user input without panics. Panic messages in the logs from the #[should_panic] tests are confusing.

#[ignore]

Do not #[ignore] tests. If the test currently does not work, assert the wrong behavior and add a fixme explaining why it is wrong.

Rationale: noticing when the behavior is fixed, making sure that even the wrong behavior is acceptable (ie, not a panic).

Exception: test data regeneration helpers in mod regenerate blocks are #[ignore]d by design. These are not behavioral tests; they regenerate expected .wasm golden files from the current compiler output and are run explicitly with --ignored when the codegen pipeline changes.

Provide Constructed Function Parameters

When writing a function, prefer to provide already constructed parameters instead of Options or similar structs. This makes the function easier to use and understand. For example, instead of something like this:

fn string_len(input: Option<String>) -> i32 {
    input.unwrap_or_default().len() as i32
}

So string_len checks if the input is available. Instead, prefer this:

fn string_len(input: String) -> i32 {
    input.len() as i32
}

So string_len always receives a valid String, and the caller is responsible for providing it.

Similarly, a function should not validate its parameters and split control flow based on that. Instead, it should assume that the parameters are within the expected range and behave accordingly. This makes the function more predictable and easier to test.

For example, instead of this:

fn divide(a: i32, b: i32) -> Option<i32> {
    if b == 0 {
        None
    } else {
        Some(a / b)
    }
}

Prefer this, where validation happens at the call site and the function assumes valid inputs:

fn divide(a: i32, b: i32) -> i32 {
    a / b
}

fn foo() {
    let x: i32 = ...;
    let y: i32 = ...;

    if y == 0 {
        panic!("Division by zero");
    }

    let result = divide(x, y);
    // Use result
}

Idiomatic Condition Checks

Use if let and match constructs for condition checks instead of manual is_some, is_none, etc. This makes the code more idiomatic and easier to read.

Type Invariants

If a type has invariants, then invariant fields must be private, set during construction, and never exposed via setters.

Useless Types

Always prefer types on the left:

// GOOD      BAD
&[T]         &Vec<T>
&str         &String
Option<&T>   &Option<T>
&Path        &PathBuf

Constructors

Prefer Default to zero-argument new function. Prefer Default even if it has to be implemented manually.

Collections

Prefer FxHashMap and FxHashSet from rustc-hash crate over standard library collections for better performance.

Error Handling in Core Crates

Each crate in core/ except cli must have a src/errors.rs file that defines a consolidated error enum for all errors that can occur within that crate. This provides structured, typed errors instead of ad-hoc string messages.

Structure

// src/errors.rs
use thiserror::Error;

#[derive(Debug, Error)]
pub enum MycrateError {
    #[error("specific error: {0}")]
    SpecificError(String),

    #[error("validation failed for {field}: {reason}")]
    ValidationError { field: String, reason: String },

    #[error("general error: {0}")]
    General(String),
}

Usage with anyhow

When returning anyhow::Result, wrap errors in the appropriate enum variant:

use anyhow::{Result, anyhow};
use crate::errors::MycrateError;

fn process_item(item: &Item) -> Result<Output> {
    if !item.is_valid() {
        return Err(anyhow!(MycrateError::ValidationError {
            field: "item".to_string(),
            reason: "item must be valid".to_string(),
        }));
    }
    // ...
}

Guidelines

  • Specific variants first: Create specific error variants for known error cases
  • Enum values hold context: Include relevant data (field names, values, positions) in enum variants
  • General fallback: Use the General(String) variant only when no specific category applies
  • Avoid bare strings: Never use anyhow!("some error") without wrapping in an error enum variant
  • CLI exception: The cli crate is a user-facing interface and should raise anyhow errors directly without a custom error enum