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

[draft] Add formatting infrastructure to the project (Sort Includes). #2524

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
106 changes: 104 additions & 2 deletions .clang-format
Original file line number Diff line number Diff line change
@@ -1,10 +1,112 @@
Language: Cpp
Standard: c++20
ColumnLimit: 100

WhitespaceSensitiveMacros:
# clang format doesn't understand TypeScript, so make sure it doesn't mangle
# overrides and additional definitions
- JSG_TS_OVERRIDE
- JSG_TS_DEFINE
- JSG_STRUCT_TS_OVERRIDE
- JSG_STRUCT_TS_DEFINE
PointerAlignment: Left
ColumnLimit: 100
AllowShortFunctionsOnASingleLine: Empty

SortIncludes: true
IncludeBlocks: Regroup
IncludeCategories:
# c++ system headers
- Regex: <[a-zA-Z0-9_]+>
Priority: 5
# kj/capnp headers
- Regex: <(kj|capnp)/.+>
Priority: 4
# 3rd party headers
- Regex: <.+>
Priority: 3
# workerd headers
- Regex: <workerd/.+>
Priority: 2
# local headers
- Regex: '".*"'
Priority: 1

AllowShortIfStatementsOnASingleLine: true
AllowShortLoopsOnASingleLine: true

IndentWidth: 2
IndentCaseBlocks: false
IndentCaseLabels: true
PointerAlignment: Left
DerivePointerAlignment: true

# Really "Attach" but empty braces aren't split.
BreakBeforeBraces: Custom
BraceWrapping:
AfterCaseLabel: false
AfterClass: false
AfterControlStatement: Never
AfterEnum: false
AfterFunction: false
AfterNamespace: false
AfterObjCDeclaration: false
AfterStruct: false
AfterUnion: false
AfterExternBlock: false
BeforeCatch: false
BeforeElse: false
BeforeLambdaBody: false
BeforeWhile: false
IndentBraces: false
SplitEmptyFunction: false
SplitEmptyRecord: false
SplitEmptyNamespace: false

Cpp11BracedListStyle: true

AlignAfterOpenBracket: DontAlign
AlignOperands: DontAlign
AlignTrailingComments:
Kind: Always
OverEmptyLines: 0
AlwaysBreakAfterReturnType: None
AlwaysBreakTemplateDeclarations: Yes
BreakStringLiterals: false
BinPackArguments: true
BinPackParameters: false
BracedInitializerIndentWidth: 2
BreakInheritanceList: BeforeColon
ContinuationIndentWidth: 4
IfMacros:
[
"KJ_SWITCH_ONEOF",
"KJ_CASE_ONEOF",
"KJ_IF_MAYBE",
"KJ_IF_SOME",
"CFJS_RESOURCE_TYPE",
]
LambdaBodyIndentation: OuterScope
Macros:
- "KJ_MAP(x,y)=[y](auto x)"
- "JSG_VISITABLE_LAMBDA(x,y,z)=[x,y](z)"
# The WhitespaceSensitiveMacros solution is flaky, adding the following ensures no formatting:
- "JSG_TS_OVERRIDE(x)=enum class"
- "JSG_TS_DEFINE(x)=enum class"
- "JSG_STRUCT_TS_OVERRIDE(x)=enum class"
- "JSG_STRUCT_TS_DEFINE(x)=enum class"
PenaltyReturnTypeOnItsOwnLine: 1000
PackConstructorInitializers: CurrentLine
ReflowComments: false
SpaceBeforeCtorInitializerColon: false
SpaceBeforeInheritanceColon: false
SpaceBeforeParens: ControlStatementsExceptControlMacros
SpaceBeforeRangeBasedForLoopColon: false
SpacesBeforeTrailingComments: 2
---
# Some files with embedded typescript are incorrectly recognized by clang-format as Objective-C
# This is because the C/C++ macro expansion step happens after the language recognition step, so
# when trying to parse the file, the c++ parser fails with incorrect syntax and a fallback to
# the Objective-C parser is used.
# This is a workaround to hide the warning.
# TODO: Remove this once we have a better solution.
Language: ObjC
DisableFormat: true
2 changes: 2 additions & 0 deletions .git-blame-ignore-revs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Apply clang-format to the project.
d17acd4d87388ff81f382ce31591e0094d32f3bd
31 changes: 31 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
name: Lint

on:
pull_request:
paths:
- src/**
- .clang-format
push:
branches:
- main

jobs:
lint:
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@v4
with:
show-progress: false
- name: Setup Linux
run: |
export DEBIAN_FRONTEND=noninteractive
wget https://apt.llvm.org/llvm.sh
sed -i '/apt-get install/d' llvm.sh
chmod +x llvm.sh
sudo ./llvm.sh 18
sudo apt-get install -y --no-install-recommends clang-format-18
- name: Lint
run: |
python3 ./tools/cross/format.py --check
env:
CLANG_FORMAT: clang-format-18
8 changes: 8 additions & 0 deletions docs/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,11 @@ just prepare
```sh
just compile-commands
```

## Code Formatting

workerd code is automatically formatted by clang-format. Run `python ./tools/cross/format.py` to reformat the code
or use the appropriate IDE extension.
While workerd generally requires llvm 15, formatting requires clang-format-18.

Code formatting is checked before check-in and during `Linting` CI build.
40 changes: 40 additions & 0 deletions githooks/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,43 @@ then
echo -e "To commit anyway, use --no-verify\n"
exit 1
fi

clang_format_check() {
source "$(dirname -- $BASH_SOURCE)/../tools/unix/find-python3.sh"
PYTHON_PATH=$(get_python3)
if [[ -z "$PYTHON_PATH" ]]; then
echo
echo "python3 is required for formatting and was not found"
echo
echo "ERROR: you must either install python3 and try pushing again or run `git push` with `--no-verify`"
return 1
fi

set +e
$PYTHON_PATH "$(dirname -- $BASH_SOURCE)/../tools/cross/format.py" --check git --staged
EXIT_CODE=$?
set -e
case $EXIT_CODE in
0)
# No lint.
return 0
;;
1)
echo
echo "ERROR: changes staged for commit have lint. Pass '--no-verify' or '-n' to skip."
echo
echo "To fix lint:"
echo " python3 ./tools/cross/format.py"
echo
return 1
;;
2)
echo
echo "ERROR: failed to run format.py, Pass '--no-verify' or '-n' to skip."
echo
return 1
;;
esac
}

clang_format_check
45 changes: 45 additions & 0 deletions githooks/pre-push
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#!/usr/bin/env bash

set -euo pipefail


source "$(dirname -- $BASH_SOURCE)/../tools/unix/find-python3.sh"
PYTHON_PATH=$(get_python3)
if [[ -z "$PYTHON_PATH" ]]; then
echo
echo "python3 is required for formatting and was not found"
echo
echo "ERROR: you must either install python3 and try pushing again or run `git push` with `--no-verify`"
exit 1
fi

while read LOCAL_REF LOCAL_SHA REMOTE_REF REMOTE_SHA
do
git fetch origin master &>/dev/null
# Check all local changes, not present in origin/master, for lint.
set +e
$PYTHON_PATH "$(dirname -- $BASH_SOURCE)/../tools/cross/format.py" --check git --source $LOCAL_SHA --target origin/master
EXIT_CODE=$?
set -e
case $EXIT_CODE in
0)
# No lint.
;;
1)
echo
echo "ERROR: changes in $LOCAL_REF have lint which may fail CI."
echo
echo "To fix lint:"
echo " python3 ./tools/cross/format.py"
echo
exit 1
;;
2)
echo
echo "ERROR: failed to run format.py, Pass '--no-verify' or '-n' to skip."
echo
exit 1
;;
esac
done

31 changes: 16 additions & 15 deletions src/workerd/api/actor-state-iocontext-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,28 @@
// Licensed under the Apache 2.0 license found in the LICENSE file or at:
// https://opensource.org/licenses/Apache-2.0

#include <algorithm>
#include <workerd/api/actor-state.h>
#include <workerd/io/actor-id.h>
#include <workerd/tests/test-fixture.h>

#include <kj/test.h>
#include <kj/encoding.h>
#include <kj/test.h>

#include <workerd/api/actor-state.h>
#include <workerd/tests/test-fixture.h>
#include <workerd/io/actor-id.h>
#include <algorithm>

namespace workerd::api {
namespace {

using workerd::TestFixture;

bool contains(kj::StringPtr haystack, kj::StringPtr needle) {
return std::search(haystack.begin(), haystack.end(), needle.begin(), needle.end())
!= haystack.end();
return std::search(haystack.begin(), haystack.end(), needle.begin(), needle.end()) !=
haystack.end();
}

class MockActorId : public ActorIdFactory::ActorId {
class MockActorId: public ActorIdFactory::ActorId {
public:
MockActorId(kj::String id) : id(kj::mv(id)) {}
MockActorId(kj::String id): id(kj::mv(id)) {}
kj::String toString() const override {
return kj::str("MockActorId<", id, ">");
}
Expand All @@ -41,6 +41,7 @@ public:
}

virtual ~MockActorId() {};

private:
kj::String id;
};
Expand All @@ -63,9 +64,9 @@ void runBadDeserialization(jsg::Lock& lock, kj::StringPtr expectedId) {

void runBadDeserializationInIoContext(TestFixture& fixture, kj::StringPtr expectedId) {
fixture.runInIoContext(
[expectedId](const workerd::TestFixture::Environment& env) -> kj::Promise<void> {
runBadDeserialization(env.lock, expectedId);
return kj::READY_NOW;
[expectedId](const workerd::TestFixture::Environment& env) -> kj::Promise<void> {
runBadDeserialization(env.lock, expectedId);
return kj::READY_NOW;
});
}

Expand All @@ -88,10 +89,10 @@ KJ_TEST("actor specified with ActorId object") {
kj::Own<ActorIdFactory::ActorId> mockActorId = kj::heap<MockActorId>(kj::str("testActorId"));
Worker::Actor::Id id = kj::mv(mockActorId);
TestFixture fixture(TestFixture::SetupParams{
.actorId = kj::mv(id),
.actorId = kj::mv(id),
});
runBadDeserializationInIoContext(fixture, "actorId = MockActorId<testActorId>;"_kj);
}

} // namespace
} // namespace workerd::api
} // namespace
} // namespace workerd::api
Loading
Loading