forked from llvm/llvm-project
-
Notifications
You must be signed in to change notification settings - Fork 1
Common Rules
June Rhodes edited this page Sep 25, 2024
·
7 revisions
The following ruleset and rules are provided for you to copy into your own project. These rules detect common issues when writing Unreal Engine C++ code.
# Append this after your own namespace/rules/rulesets, including the '---' separator.
---
Namespace: llvm.redpoint.games
Rulesets:
- Name: clang-tidy
Severity: Error
Rules:
- performance-unnecessary-value-param
Rules:
- # Find function parameters that could be 'const &'.
# @note: We exclude functions starting with 'On' because we assume they might
# have delegate captures that must not be passed by reference.
Name: performance-unnecessary-value-param
Matcher: |
functionDecl(
hasBody(stmt()),
isDefinition(),
unless(isImplicit()),
unless(matchesName("::On.*")),
unless(cxxMethodDecl(anyOf(isOverride(), isFinal()))),
has(typeLoc(forEach(
parmVarDecl(
hasType(qualType(
hasCanonicalType(isExpensiveToCopy()),
unless(hasCanonicalType(referenceType())))),
decl().bind("param")
)
))),
unless(isInstantiated()), decl().bind("functionDecl")
)
ErrorMessage: |
parameter should be made 'const &' to avoid unnecessary copy
Callsite: param# Append this after your own namespace/rules/rulesets, including the '---' separator.
---
Namespace: unreal.redpoint.games
Rulesets:
- Name: unreal-engine
Severity: Error
Rules:
- Name: ionlinesubsystem-get
Severity: Warning
- Name: unsafe-storage-of-oss-pointer
Severity: Warning
- broken-array-call
- bad-reference-capture-in-delegate
- missing-uproperty
- missing-super-call-lifetimeprops
- missing-doreplifetime-for-replicated-property
- incorrect-interface-invocation
- ustruct-field-not-initialized-in-class
# Enable these rules if you're building plugins for the Marketplace:
- ufunction-must-have-category
- uproperty-must-have-category
Rules:
- # Detects usages of array functions where the item parameter is a reference
# back into the array, and where the reference originates from a for-range
# loop.
#
# This detects bad code like this:
#
# for (const auto& Val : Arr)
# {
# Arr.Remove(Val);
# }
#
# which is invalid because the array will free the memory that Val is
# pointing to. Unreal checks this at runtime and will assert; this clang-tidy
# rule detects it at compile time.
#
# To fix this code, make the iteration value a copy of the value
# from the array, or index into the array instead of using a for-range loop:
#
# for (auto Val : Arr)
# {
# Arr.Remove(Val);
# }
Name: broken-array-call
Matcher: |
cxxMemberCallExpr(
on(
declRefExpr(
hasType(cxxRecordDecl(hasName("TArray"))),
to(decl().bind("array_declared_here"))
).bind("array_callsite")
),
callee(
cxxMethodDecl(
matchesName("(Insert|Insert_GetRef|Add|Add_GetRef|Remove|RemoveSwap)")
)
),
hasArgument(
0,
declRefExpr(
to(
varDecl(
hasType(referenceType()),
hasAncestor(
cxxForRangeStmt(
hasRangeInit(
declRefExpr(
to(decl(equalsBoundNode("array_declared_here")))
)
)
).bind("array_for_range")
)
).bind("dangerous_ref_declaration")
)
).bind("dangerous_ref_usage")
)
).bind("bad_callsite")
ErrorMessage: |
incorrect usage of mutating array call with non-copy will lead to crash at runtime
Callsite: bad_callsite
Hints:
array_for_range: "make this a copy instead of a ref (i.e. not const&), or switch to an index-based for loop"
- # Detects when you capture a reference as a user parameter to a delegate. This is almost always incorrect, as the delegate will capture the reference, not the value. When the referenced memory goes out of scope, the delegate can be invoked with arguments that point to an invalid memory space.
#
# For example:
#
# DECLARE_DELEGATE(FSomeFunctionHandler)
# void FunctionHandler(const int& UserParam) {};
# void BadImpl()
# {
# int A = 5;
# FSomeFunctionHandler::CreateStatic(&FunctionHandler, A);
# // If CreateStatic was passed to an event to be called after BadImpl
# // returns, it would be invoked with UserParam pointing to invalid memory.
# }
#
# You should always pass user parameters by value.
Name: bad-reference-capture-in-delegate
Matcher: |
declRefExpr(to(functionDecl(hasName("(Create|Bind)(Lambda|Raw|SP|Static|ThreadSafeSP|UFunction|UObject|WeakLambda)"), isTemplateInstantiation(), hasAnyTemplateArgument(refersToPack(refersToType(lValueReferenceType().bind("captured_reference"))))))).bind("bad_callsite")
ErrorMessage: |
creating or binding of delegate incorrectly captures reference
Callsite: bad_callsite
Hints:
captured_reference: "this is the reference that is incorrectly captured. change the function declaration so this is passed by value instead."
- # Detects when you forget to call Super::BeginDestroy(); in an overridden BeginDestroy function.
Name: missing-super-call-begindestroy
Matcher: |
cxxMethodDecl(
isOverride(),
hasName("BeginDestroy"),
hasBody(compoundStmt()),
unless(
hasDescendant(
cxxMemberCallExpr(
callee(cxxMethodDecl(hasName("BeginDestroy"))),
on(cxxThisExpr())
)
)
)
).bind("bad_impl")
ErrorMessage: |
missing call to Super::BeginDestroy(). make sure you call Super::BeginDestroy() in your overridden implementation.
Callsite: bad_impl
- # Detects when you use IOnlineSubsystem::Get(). You should always use Online::GetSubsystem() instead.
Name: ionlinesubsystem-get
Matcher: |
callExpr(
callee(
cxxMethodDecl(
ofClass(
cxxRecordDecl(
hasName("IOnlineSubsystem")
)
),
hasName("Get")
)
)
).bind("bad_callsite")
ErrorMessage: |
use Online::GetSubsystem(this->GetWorld()) instead of IOnlineSubsystem::Get(), as IOnlineSubsystem::Get() can't be safely used in the editor.
Callsite: bad_callsite
- # Detects when you try to store an IOnlineSubsystemPtr or IOnline*Ptr as a field in a class or struct.
#
# You can not safely store these values, as they will prevent the online subsystem from releasing it's resources, which it may do when the editor is open and play-in-editor is started or stopped.
#
# You should use weak pointers e.g. TWeakPtr<IOnlineIdentity, ESPMode::ThreadSafe> instead.
Name: unsafe-storage-of-oss-pointer
Matcher: |
fieldDecl(
hasType(
hasUnqualifiedDesugaredType(
recordType(
hasDeclaration(
classTemplateSpecializationDecl(
hasName("TSharedPtr"),
hasTemplateArgument(
0,
refersToType(
hasDeclaration(
namedDecl(
matchesName("IOnline[A-Za-z0-9]+")
)
)
)
)
)
)
)
)
)
).bind("bad_field")
ErrorMessage: |
storing shared pointers of this type at the struct or class level is unsafe, as it will prevent the online subsystem from safely releasing it's resources. use a TWeakPtr<> instead.
Callsite: bad_field
- # Detects when you forget to add UPROPERTY() to fields in a UCLASS(), where those fields point to other garbage collected objects.
Name: missing-uproperty
Matcher: |
fieldDecl(
unless(
isUProperty()
),
hasParent(
cxxRecordDecl(
isUClass()
)
),
hasType(
pointerType(
pointee(
recordType(
hasDeclaration(
cxxRecordDecl(
isUClass()
)
)
)
)
)
)
).bind("bad_field")
ErrorMessage: |
missing UPROPERTY() declaration on this field, which is required for all UObject pointers
Callsite: bad_field
- # Detects when you forget to call Super::GetLifetimeReplicatedProps(); in an overridden GetLifetimeReplicatedProps function.
Name: missing-super-call-lifetimeprops
Matcher: |
cxxMethodDecl(
isOverride(),
hasName("GetLifetimeReplicatedProps"),
hasBody(compoundStmt()),
ofClass(
cxxRecordDecl(
isDirectlyDerivedFrom(
typedefNameDecl(
hasType(
type().bind("parent_class")
)
)
)
)
),
unless(
hasDescendant(
cxxMemberCallExpr(
callee(
cxxMethodDecl(
hasName("GetLifetimeReplicatedProps")
)
),
anyOf(
thisPointerType(
qualType(
recordType(
equalsBoundNode("parent_class")
)
)
),
onImplicitObjectArgument(
implicitCastExpr(
has(
implicitCastExpr(
hasImplicitDestinationType(
qualType(
pointsTo(
typedefType(
hasDeclaration(
typedefDecl(
hasName("Super")
)
)
)
)
)
)
)
)
)
)
)
)
)
)
).bind("bad_impl")
ErrorMessage: |
missing call to Super::GetLifetimeReplicatedProps(). make sure you call Super::GetLifetimeReplicatedProps() in your overridden implementation.
Callsite: bad_impl
- # Detects when you forget to call DOREPLIFETIME() for a replicated property.
Name: missing-doreplifetime-for-replicated-property
Matcher: |
cxxMethodDecl(
hasName("GetLifetimeReplicatedProps"),
ofClass(
cxxRecordDecl(
forEach(
fieldDecl(
isUProperty(),
hasUSpecifier("replicated")
).bind("declared_property")
)
)
),
hasBody(
compoundStmt(
forNoDescendant(
callExpr(
callee(
namedDecl(
hasName("GetMemberNameCheckedJunk")
)
),
hasArgument(
0,
memberExpr(
member(
fieldDecl(
equalsBoundNode("declared_property")
)
)
)
)
)
)
)
)
).bind("configuration_site")
ErrorMessage: |
missing call to DOREPLIFETIME() or equivalent macro in GetLifetimeReplicatedProps for a replicated property.
Callsite: declared_property
Hints:
configuration_site: "this implementation needs to call DOREPLIFETIME() or an similar macro to configure this property for replication"
- # Detects when you call an IInterface method directly with I->Test(); instead of using IInterface::Execute_Test(I);
Name: incorrect-interface-invocation
Matcher: |
cxxMemberCallExpr(
thisPointerType(
hasDeclaration(
cxxRecordDecl(
isIInterface()
)
)
),
callee(
cxxMethodDecl(
isUFunction(),
anyOf(
hasUSpecifier("BlueprintImplementableEvent"),
hasUSpecifier("BlueprintNativeEvent")
)
)
)
).bind("bad_callsite")
ErrorMessage: |
incorrect call to UInterface member method. you must call methods via IInterface::Execute_Func(Val, Arg1, Arg2, ...) instead of Val->Func(Arg1, Arg2, ...).
Callsite: bad_callsite
- # Detects when a USTRUCT property does not have an in-class initializer. This causes a runtime warning as of Unreal Engine 5 and is marked to cause a runtime error in future, so this check catches the issue at compile time.
Name: ustruct-field-not-initialized-in-class
Matcher: |
fieldDecl(
hasType(qualType(isPODType())),
unless(hasType(cxxRecordDecl(isUStruct()))),
unless(hasInClassInitializer(expr())),
hasDeclContext(cxxRecordDecl(isUStruct()))
).bind("bad_field")
ErrorMessage: |
missing default initializer for POD-typed field declaration in USTRUCT (expected a zeroing initializer like 'int A = 0;' or 'bool B = false;')
Callsite: bad_field
- # Detects when a UFUNCTION is missing the 'Category' specifier, which is required for plugins that are submitting to the Marketplace/Fab.
Name: ufunction-must-have-category
Matcher: |
namedDecl(
isUFunction(),
anyOf(
hasUSpecifier("BlueprintCallable"),
hasUSpecifier("BlueprintPure")),
unless(anyOf(
hasUMetadata("Category"),
hasUSpecifier("Category"))),
unless(hasUSpecifier("BlueprintSetter")),
unless(hasUSpecifier("BlueprintGetter")),
unless(hasUSpecifier("BlueprintInternalUseOnly")),
unless(hasUSpecifier("DeprecatedFunction"))
).bind("decl")
ErrorMessage: |
UFUNCTION is missing 'Category' specifier
Callsite: decl
- # Detects when a UPROPERTY is missing the 'Category' specifier, which is required for plugins that are submitting to the Marketplace/Fab.
Name: uproperty-must-have-category
Matcher: |
namedDecl(
isUProperty(),
anyOf(
hasUSpecifier("BlueprintVisible"),
hasUSpecifier("EditAnywhere"),
hasUSpecifier("EditDefaultsOnly"),
hasUSpecifier("EditInstanceOnly")),
unless(anyOf(
hasUMetadata("Category"),
hasUSpecifier("Category")))
).bind("decl")
ErrorMessage: |
UPROPERTY is missing 'Category' specifier
Callsite: decl