Skip to content

Commit

Permalink
Clang tidy (#224)
Browse files Browse the repository at this point in the history
* Don't use relative paths in include directories

* clang-tidy fixes

* clang-tidy fixes

* Added a clang-tidy driver and a minimal .clang-tidy config
  • Loading branch information
mikekazakov authored Apr 22, 2024
1 parent 7281635 commit 82b4678
Show file tree
Hide file tree
Showing 41 changed files with 566 additions and 713 deletions.
90 changes: 90 additions & 0 deletions Scripts/run_clang_tidy.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
#!/bin/sh

set -e
set -o pipefail

if ! [ -x "$(command -v xcpretty)" ] ; then
echo 'xcpretty is not found, aborting. (https://github.com/xcpretty/xcpretty)'
exit -1
fi

if ! [ -x "$(command -v /usr/local/opt/llvm/bin/clang-tidy)" ] ; then
echo 'clang-tidy is not found, aborting. Do brew install llvm.'
exit -1
fi

if ! [ -x "$(command -v jq)" ] ; then
echo 'jq is not found, aborting. Do brew install jq.'
exit -1
fi

# https://github.com/xcpretty/xcpretty/issues/48
export LC_CTYPE=en_US.UTF-8

# Tools from LLVM
RUNTIDY=/usr/local/opt/llvm/bin/run-clang-tidy
TIDY=/usr/local/opt/llvm/bin/clang-tidy
APPLY=/usr/local/opt/llvm/bin/clang-apply-replacements

# Get current directory
SCRIPTS_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
ROOT_DIR=$(cd "$SCRIPTS_DIR/.." && pwd)

# Allocate a dir for build artifacts
BUILD_DIR="${SCRIPTS_DIR}/run_clang_tidy.tmp"
mkdir -p "${BUILD_DIR}"
xattr -w com.apple.xcode.CreatedByBuildSystem true "${BUILD_DIR}"

# A project to build
XCODEPROJ="${ROOT_DIR}/Source/NimbleCommander/NimbleCommander.xcodeproj"

# Compose a list of targets to build
uts=$(xcodebuild -project ${XCODEPROJ} -list | awk -v word="Schemes:" 'BEGIN {found=0} found {if ($0 ~ /UT$/) print} $0 ~ word {found=1}' | sed 's/^[[:space:]]*//')
its=$(xcodebuild -project ${XCODEPROJ} -list | awk -v word="Schemes:" 'BEGIN {found=0} found {if ($0 ~ /IT$/) print} $0 ~ word {found=1}' | sed 's/^[[:space:]]*//')
others=("NimbleCommander-Unsigned" "RoutedIO" "info.filesmanager.Files.PrivilegedIOHelperV2")
targets=("${uts[@]}" "${its[@]}" "${others[@]}")
echo Building these targets: ${targets[@]}

# Common invocation of xcodebuild
XC="xcodebuild \
-project ${XCODEPROJ} \
-configuration Debug \
SYMROOT=${BUILD_DIR} \
OBJROOT=${BUILD_DIR} \
ONLY_ACTIVE_ARCH=YES \
COMPILER_INDEX_STORE_ENABLE=NO"

# Clean each target
for target in ${targets[@]}; do
$XC -scheme ${target} clean
done

# Build each target
echo "[]" > compile_commands.json
for target in ${targets[@]}; do
# Build the target and capture the invocation command
$XC -scheme ${target} build | xcpretty -r json-compilation-database --output compile_commands-${target}.json

# Remove the "-ivfsstatcache path" flags that clang-tidy doesn't understand
sed -i '' 's/-ivfsstatcache[^\"]*\.sdkstatcache//g' "compile_commands-${target}.json"

# Temporarily combine the new JSON with the accumulating compile_commands.json
jq -s 'add' compile_commands.json "compile_commands-${target}.json" > temp_compile_commands.json

# Replace the main compile_commands.json with the updated version
mv temp_compile_commands.json compile_commands.json

# The compile commands of this target
rm "compile_commands-${target}.json"
done

# Run clang-tidy in parallel via run-clang-tidy
${RUNTIDY} \
-p ${SCRIPTS_DIR} \
-clang-tidy-binary ${TIDY} \
-clang-apply-replacements-binary ${APPLY} \
-j $(sysctl -n hw.activecpu) \
-use-color 1 \
-fix \
-format \
"${ROOT_DIR}/Source/.*"
7 changes: 7 additions & 0 deletions Source/.clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Checks: >
-*,
-clang-diagnostic-c99-designator,
modernize-avoid-bind
ExtraArgs:
- -Wno-unknown-pragmas
2 changes: 1 addition & 1 deletion Source/Base/source/chained_strings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace nc::base {

chained_strings::chained_strings() : m_Begin(nullptr), m_Last(nullptr)
{
static_assert(sizeof(node) == 24, "size of string node should be 14 bytes");
static_assert(sizeof(node) == 24, "size of string node should be 24 bytes");
static_assert(sizeof(block) == 1024, "size of strings chunk should be 1024 bytes");
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (C) 2015-2023 Michael Kazakov. Subject to GNU General Public License version 3.
// Copyright (C) 2015-2024 Michael Kazakov. Subject to GNU General Public License version 3.
#include "ConfigBackedNetworkConnectionsManager.h"
#include <dirent.h>
#include <NetFS/NetFS.h>
Expand Down Expand Up @@ -264,7 +264,7 @@ static void SortByMRU(std::vector<NetworkConnectionsManager::Connection> &_value
// Wire up on-the-fly loading of externally changed config
m_Config.ObserveMany(
m_ConfigObservations,
[=] {
[this] {
if( !m_IsWritingConfig )
Load();
},
Expand All @@ -285,7 +285,7 @@ static void SortByMRU(std::vector<NetworkConnectionsManager::Connection> &_value
else
m_Connections.emplace_back(_conn);
}
dispatch_to_background([=] { Save(); });
dispatch_to_background([this] { Save(); });
}

void ConfigBackedNetworkConnectionsManager::RemoveConnection(const Connection &_connection)
Expand All @@ -301,7 +301,7 @@ static void SortByMRU(std::vector<NetworkConnectionsManager::Connection> &_value
if( i != end(m_MRU) )
m_MRU.erase(i);
}
dispatch_to_background([=] { Save(); });
dispatch_to_background([this] { Save(); });
}

std::optional<NetworkConnectionsManager::Connection>
Expand Down Expand Up @@ -368,7 +368,7 @@ static void SortByMRU(std::vector<NetworkConnectionsManager::Connection> &_value
else
m_MRU.insert(begin(m_MRU), _connection.Uuid());
}
dispatch_to_background([=] { Save(); });
dispatch_to_background([this] { Save(); });
}

std::vector<NetworkConnectionsManager::Connection> ConfigBackedNetworkConnectionsManager::FTPConnectionsByMRU() const
Expand Down
72 changes: 32 additions & 40 deletions Source/NimbleCommander/NimbleCommander/Core/SandboxManager.mm
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (C) 2014-2023 Michael Kazakov. Subject to GNU General Public License version 3.
// Copyright (C) 2014-2024 Michael Kazakov. Subject to GNU General Public License version 3.
#include <sys/stat.h>
#include <Base/algo.h>
#include <Base/CommonPaths.h>
Expand Down Expand Up @@ -71,13 +71,12 @@ - (BOOL)panel:(id) [[maybe_unused]] _sender shouldEnableURL:(NSURL *)_url
SandboxManager::SandboxManager()
{
LoadSecurityScopeBookmarks_Unlocked();
[NSNotificationCenter.defaultCenter
addObserverForName:NSApplicationWillTerminateNotification
object:NSApplication.sharedApplication
queue:NSOperationQueue.mainQueue
usingBlock:^([[maybe_unused]] NSNotification *note) {
SandboxManager::Instance().StopUsingBookmarks();
}];
[NSNotificationCenter.defaultCenter addObserverForName:NSApplicationWillTerminateNotification
object:NSApplication.sharedApplication
queue:NSOperationQueue.mainQueue
usingBlock:^([[maybe_unused]] NSNotification *note) {
SandboxManager::Instance().StopUsingBookmarks();
}];
}

SandboxManager &SandboxManager::Instance()
Expand All @@ -90,20 +89,18 @@ - (BOOL)panel:(id) [[maybe_unused]] _sender shouldEnableURL:(NSURL *)_url
{
assert(m_Bookmarks.empty());

auto bookmarks =
nc::objc_cast<NSArray>([NSUserDefaults.standardUserDefaults objectForKey:g_BookmarksKey]);
auto bookmarks = nc::objc_cast<NSArray>([NSUserDefaults.standardUserDefaults objectForKey:g_BookmarksKey]);
if( !bookmarks )
return;

for( id obj : bookmarks )
if( auto *data = nc::objc_cast<NSData>(obj) ) {
NSURL *scoped_url =
[NSURL URLByResolvingBookmarkData:data
options:NSURLBookmarkResolutionWithoutMounting |
NSURLBookmarkResolutionWithSecurityScope
relativeToURL:nil
bookmarkDataIsStale:nil
error:nil];
NSURL *scoped_url = [NSURL URLByResolvingBookmarkData:data
options:NSURLBookmarkResolutionWithoutMounting |
NSURLBookmarkResolutionWithSecurityScope
relativeToURL:nil
bookmarkDataIsStale:nil
error:nil];
if( scoped_url ) {
// check that scoped_url is still valid
if( [scoped_url startAccessingSecurityScopedResource] ) {
Expand Down Expand Up @@ -140,8 +137,7 @@ - (BOOL)panel:(id) [[maybe_unused]] _sender shouldEnableURL:(NSURL *)_url
{
if( !nc::dispatch_is_main_queue() ) {
bool result = false;
dispatch_sync(dispatch_get_main_queue(),
[&] { result = AskAccessForPathSync(_path, _mandatory_path); });
dispatch_sync(dispatch_get_main_queue(), [&] { result = AskAccessForPathSync(_path, _mandatory_path); });
return result;
}

Expand All @@ -155,16 +151,15 @@ - (BOOL)panel:(id) [[maybe_unused]] _sender shouldEnableURL:(NSURL *)_url
forKey:@"NSNavLastRootDirectory"];

NSOpenPanel *openPanel = NSOpenPanel.openPanel;
openPanel.message = NSLocalizedString(
@"Click “Allow Access” to grant access to files in the selected directory",
"Asking the user to grant filesystem access for NC");
openPanel.prompt = NSLocalizedString(
@"Allow Access", "Asking user for granting file system access for NC - button title");
openPanel.message = NSLocalizedString(@"Click “Allow Access” to grant access to files in the selected directory",
"Asking the user to grant filesystem access for NC");
openPanel.prompt =
NSLocalizedString(@"Allow Access", "Asking user for granting file system access for NC - button title");
openPanel.canChooseFiles = false;
openPanel.canChooseDirectories = true;
openPanel.allowsMultipleSelection = false;
SandboxManagerPanelDelegate *delegate =
[[SandboxManagerPanelDelegate alloc] initWithPath:reqired_path mandatory:_mandatory_path];
SandboxManagerPanelDelegate *delegate = [[SandboxManagerPanelDelegate alloc] initWithPath:reqired_path
mandatory:_mandatory_path];
openPanel.delegate = delegate;
openPanel.directoryURL = [NSURL fileURLWithFileSystemRepresentation:reqired_path.c_str()
isDirectory:true
Expand All @@ -173,18 +168,16 @@ - (BOOL)panel:(id) [[maybe_unused]] _sender shouldEnableURL:(NSURL *)_url
const auto res = [openPanel runModal];
if( res == NSModalResponseOK )
if( NSURL *url = openPanel.URL ) {
NSData *bookmark_data =
[url bookmarkDataWithOptions:NSURLBookmarkCreationWithSecurityScope
includingResourceValuesForKeys:nil
relativeToURL:nil
error:nil];
NSData *bookmark_data = [url bookmarkDataWithOptions:NSURLBookmarkCreationWithSecurityScope
includingResourceValuesForKeys:nil
relativeToURL:nil
error:nil];
if( bookmark_data ) {
NSURL *scoped_url =
[NSURL URLByResolvingBookmarkData:bookmark_data
options:NSURLBookmarkResolutionWithSecurityScope
relativeToURL:nil
bookmarkDataIsStale:nil
error:nil];
NSURL *scoped_url = [NSURL URLByResolvingBookmarkData:bookmark_data
options:NSURLBookmarkResolutionWithSecurityScope
relativeToURL:nil
bookmarkDataIsStale:nil
error:nil];
if( scoped_url && [scoped_url startAccessingSecurityScopedResource] ) {
Bookmark bm;
bm.data = bookmark_data;
Expand All @@ -195,7 +188,7 @@ - (BOOL)panel:(id) [[maybe_unused]] _sender shouldEnableURL:(NSURL *)_url
m_Bookmarks.emplace_back(bm);
}

dispatch_to_background([=] { SaveSecurityScopeBookmarks(); });
dispatch_to_background([this] { SaveSecurityScopeBookmarks(); });

return HasAccessToFolder_Unlocked(_path);
}
Expand Down Expand Up @@ -277,8 +270,7 @@ - (BOOL)panel:(id) [[maybe_unused]] _sender shouldEnableURL:(NSURL *)_url

bool SandboxManager::EnsurePathAccess(const std::string &_path)
{
if( !SandboxManager::Instance().CanAccessFolder(_path) &&
!SandboxManager::Instance().AskAccessForPathSync(_path) )
if( !SandboxManager::Instance().CanAccessFolder(_path) && !SandboxManager::Instance().AskAccessForPathSync(_path) )
return false;
return true;
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (C) 2016-2023 Michael Kazakov. Subject to GNU General Public License version 3.
// Copyright (C) 2016-2024 Michael Kazakov. Subject to GNU General Public License version 3.
#include "TemporaryNativeFileChangesSentinel.h"
#include <Base/algo.h>
#include <Base/Hash.h>
Expand Down Expand Up @@ -52,9 +52,8 @@ bool TemporaryNativeFileChangesSentinel::WatchFile(const std::string &_path,
auto current = std::make_shared<Meta>();
auto &dir_update = nc::utility::FSEventsDirUpdate::Instance();
const auto path = std::filesystem::path(_path).parent_path();
uint64_t watch_ticket = dir_update.AddWatchPath(path.c_str(), [current] {
TemporaryNativeFileChangesSentinel::Instance().FSEventCallback(current);
});
uint64_t watch_ticket = dir_update.AddWatchPath(
path.c_str(), [current] { TemporaryNativeFileChangesSentinel::Instance().FSEventCallback(current); });
if( !watch_ticket )
return false;

Expand All @@ -77,7 +76,7 @@ void TemporaryNativeFileChangesSentinel::ScheduleItemDrop(const std::shared_ptr<
using namespace std::chrono;
static const auto safety_backlash = 100ms;
_meta->drop_time = duration_cast<milliseconds>(nc::base::machtime() + _meta->drop_delay);
dispatch_to_background_after(_meta->drop_delay + safety_backlash, [=] {
dispatch_to_background_after(_meta->drop_delay + safety_backlash, [=, this] {
if( _meta->drop_time < nc::base::machtime() )
StopFileWatch(_meta->path);
});
Expand All @@ -87,8 +86,7 @@ bool TemporaryNativeFileChangesSentinel::StopFileWatch(const std::string &_path)
{
auto &dir_update = nc::utility::FSEventsDirUpdate::Instance();
auto lock = std::lock_guard{m_WatchesLock};
auto it = find_if(
begin(m_Watches), end(m_Watches), [&](const auto &_i) { return _i->path == _path; });
auto it = find_if(begin(m_Watches), end(m_Watches), [&](const auto &_i) { return _i->path == _path; });
if( it != end(m_Watches) ) {
auto meta = *it;
dir_update.RemoveWatchPathWithTicket(meta->fswatch_ticket);
Expand All @@ -107,7 +105,7 @@ void TemporaryNativeFileChangesSentinel::FSEventCallback(std::shared_ptr<Meta> _

_meta->checking_now = true;

dispatch_to_background_after(_meta->check_delay, [=] { BackgroundItemCheck(_meta); });
dispatch_to_background_after(_meta->check_delay, [=, this] { BackgroundItemCheck(_meta); });
}

void TemporaryNativeFileChangesSentinel::BackgroundItemCheck(std::shared_ptr<Meta> _meta)
Expand Down
Loading

0 comments on commit 82b4678

Please sign in to comment.