Files
ResInsight/docs/agents/coding-style.md
T
Magne Sjaastad 21694b11e7 #14476 Align CAF_ASSERT with standard assert semantics
CAF_ASSERT was unconditionally active in every build configuration. Make it
follow the semantics of the standard assert(): active in Debug, compiled out in
optimized builds (NDEBUG).

Add the CMake option RESINSIGHT_ENABLE_ASSERTS_IN_RELEASE (default OFF) for
developers who want the asserts to stay active in an optimized build, which is
useful when reproducing a problem in RelWithDebInfo with a debugger attached.

When compiled out, the expression is kept inside an unevaluated sizeof rather
than discarded. It is not evaluated, so there is no run-time cost and no side
effects, but it is still type checked and any variable used only by the assert
still counts as referenced, avoiding a wave of unused-variable warnings.

CAF_ENABLE_ASSERTS is given a default in cafAssert.h so the header stays
self-contained and include order can never silently switch the asserts off.

Document in docs/agents/coding-style.md that CAF_ASSERT is the assert to use,
that CVF_ASSERT is legacy, and that asserts are for broken invariants rather
than for run-time conditions that need real error handling.
2026-08-07 15:05:57 +02:00

7.0 KiB
Raw Blame History

Coding Style Guidelines

This document describes the coding style and formatting conventions for ResInsight.

Code Formatting

clang-format

ResInsight uses clang-format for C++ code formatting:

  • Configuration: .clang-format file in repository root
  • Version: Use clang-format-19 to enforce style

Python Formatting

Python code should be formatted using ruff:

# Format source code
python -m ruff format test_polygons.py 

# Check code style
python -m ruff check --fix test_polygons.py
  • New files must use the current year (the year the file is created) in the copyright header
  • Never change the copyright year in existing files — the year reflects when the file was originally created
  • Example header for a file created in 2026:
/////////////////////////////////////////////////////////////////////////////////
//
//  Copyright (C) 2026-     Equinor ASA
//

Language Standards

  • C++: C++23 standard
  • Python: Python 3.11+

Best Practices

General Guidelines

  1. Minimal Changes: Make the smallest possible changes to achieve the goal
  2. Preserve Formatting: Do not reformat unrelated code
  3. Comments: Match the style of existing comments in the file
  4. Libraries: Use existing libraries whenever possible; only add new libraries or update versions if absolutely necessary

Code Quality

  1. Always validate changes don't break existing behavior
  2. Always validate changes don't introduce security vulnerabilities
  3. Fix any vulnerabilities related to your changes
  4. Run linters, builds and tests before making code changes to understand any existing issues
  5. Always try to lint, build and test code changes as soon as possible after making them

Testing

  1. Only run linters, builds and tests that already exist
  2. Do not add new linting, building or testing tools unless necessary to fix the issue
  3. It is unacceptable to remove or edit unrelated tests
  4. Documentation changes do not need to be linted, built or tested unless there are specific tests for documentation

Header / Implementation Split (C++)

Put function bodies in the .cpp file, not inline in the header.

  • Declare member functions in the header; define them in the matching .cpp.
  • This applies even to one-line bodies (return false;, return m_field;, trivial forwarders).
  • Exceptions: function templates that must stay in the header, and constexpr functions where the compiler requires the definition to be visible.
// Bad  inline body in the header
class RimFoo
{
public:
    bool canAddSubCollection() const override { return false; }
};

// Good  declared in the header, defined in the .cpp
// RimFoo.h
class RimFoo
{
public:
    bool canAddSubCollection() const override;
};

// RimFoo.cpp
bool RimFoo::canAddSubCollection() const
{
    return false;
}

Why: keeps headers light (faster builds, smaller include surface), keeps the implementation file as the single source of truth for behavior, and matches the existing style across ApplicationLibCode.

Logging

Use std::format to build log messages — not QString(...).arg(...).

  • RiaLogging::info / warning / error take std::string_view. Passing a QString requires a trailing .toStdString(); std::format avoids that and reads better.
  • QString arguments can be passed directly to std::format — include RiaQStringFormatter.h, which provides a std::formatter<QString> specialization.
// Bad  QString with positional placeholders, manual conversion
RiaLogging::info( QString( "Exported %1 cells for well '%2'." ).arg( count ).arg( wellName ).toStdString() );

// Good  std::format, QString passed directly
RiaLogging::info( std::format( "Exported {} cells for well '{}'.", count, wellName ) );

Asserts

Use CAF_ASSERT (cafAssert.h). Do not add new CVF_ASSERT, CVF_ASSERT_MSG, CVF_FAIL_MSG or CVF_TIGHT_ASSERT — those are legacy and are being migrated away from.

An assert documents an invariant that holds whenever the program is correct. It is not error handling. Conditions that can legitimately occur at run time — a missing file, a malformed input deck, a failed network call, a user selecting nothing — must be handled with a return value, std::optional, std::expected or a logged error message, never with an assert.

// Bad  a missing file is a run-time condition, not a broken invariant
CAF_ASSERT( QFile::exists( fileName ) );

// Good  handled and reported
if ( !QFile::exists( fileName ) )
{
    RiaLogging::error( std::format( "File not found: {}", fileName ) );
    return {};
}

// Good  a broken invariant, unreachable unless the code is wrong
CAF_ASSERT( index >= 0 && index < m_values.size() );

CAF_ASSERT follows the semantics of the standard assert():

  • Debug builds: active. A failure prints file, line and expression, then calls std::abort().
  • Optimized builds (Release, RelWithDebInfo): compiled out. The expression is still type checked but never evaluated, so it must be free of side effects — never put work inside the assert that the surrounding code depends on.

Configure with -DRESINSIGHT_ENABLE_ASSERTS_IN_RELEASE=ON to keep the asserts active in an optimized build, which is useful when reproducing a problem in RelWithDebInfo with a debugger attached.

A failing assert aborts via SIGABRT, which the crash handler installed in RiaMain.cpp picks up: the failure is written to the log file and reported to OpenTelemetry with a stack trace.

Lambda Functions

Keep lambdas short and readable:

  • 13 lines: keep inline
  • 4+ lines or contains control flow (if/for/while): extract to a named method (e.g. onApplyClicked()) and call it from a single-line lambda
// Good  short inline lambda
addNewButton( "Show Report", [this]() { showReport(); } );

// Good  multi-statement, no control flow, still inline
addNewButton( "Clear Data",
              [this]()
              {
                  clearData();
                  updateConnectedEditors();
              } );

// Good  complex logic extracted to a named method
addNewButton( "Apply", [this]() { onApplyClicked(); } );
void MyClass::onApplyClicked() { /* complex logic here */ }

Context Menus

When adding or modifying context-menu items for a PDM object, override appendMenuItems( caf::CmdFeatureMenuBuilder& menuBuilder ) on that object and add the entries there.

  • Do not modify RimContextCommandBuilder to add object-specific menu items. It dispatches to each object's appendMenuItems; keep object-specific logic in the object.
// Good  menu items added in the object's own appendMenuItems override
void RimFoo::appendMenuItems( caf::CmdFeatureMenuBuilder& menuBuilder ) const
{
    menuBuilder << "RicSomeFeature";
}

Commit Conventions

When creating commits:

  • Use issue number at the start of the title: #12773 Python: Add API for creating valve templates
  • Follow git conventions for commit messages
  • Always run python formatting/check on changed files before commits