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

Clang tidy - apply #326

Merged
merged 25 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
c2efbd3
Add plural rules to CPP
sven-oly May 21, 2024
69aa913
Merge remote-tracking branch 'upstream/main'
sven-oly May 23, 2024
097c277
Merge remote-tracking branch 'upstream/main'
sven-oly May 23, 2024
ca2d4a5
Merge remote-tracking branch 'upstream/main'
sven-oly May 28, 2024
d790a5d
Clang-tidy applied to *.cpp
sven-oly May 28, 2024
756fdb6
More style fixes
sven-oly May 29, 2024
83d9a69
Merging
sven-oly Oct 10, 2024
16a3cb3
DDT-234 Apply clang-tidy to *.cpp
sven-oly Oct 10, 2024
9325c23
Merge branch 'unicode-org:main' into clang_tidy
sven-oly Oct 11, 2024
c79666c
Added Clang Tidy to CI
echeran Nov 5, 2024
2c6e654
Apply changes from clang-tidy
echeran Nov 6, 2024
3cf0b62
Make icu4c setup script more robust
echeran Nov 6, 2024
acb1cca
Add a clang `compile_commands.txt` to assist compilation output msgs
echeran Nov 6, 2024
af0ea03
Apply clang-tidy change
echeran Nov 6, 2024
89908ac
Add clang-tidy config file
echeran Nov 6, 2024
96b4326
Test clang-tidy run on CI to sidestep CI upload step permissions
echeran Nov 6, 2024
0e64096
Mod to workaround GHA permissions
echeran Nov 6, 2024
c2a6087
Try to include json-c dependency in clang-tidy CI action
echeran Nov 6, 2024
d6b1d99
Refactor clang-tidy CI jobs into a separate workflow
echeran Nov 6, 2024
4b39c35
Update CI job names
echeran Nov 6, 2024
697d455
Change workflow name to trigger the 2nd workflow
echeran Nov 6, 2024
2053069
Remove second workflow b/c it won't trigger without user PAT tokens o…
echeran Nov 6, 2024
d7a90ff
test CI by inducing a known error
echeran Nov 6, 2024
072cdfa
Revert "test CI by inducing a known error"
echeran Nov 6, 2024
6fccc83
Add CLI usage for clang-tidy locally
echeran Nov 6, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .github/workflows/clang-tidy-review.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
name: clang-tidy-review

# You can be more specific, but it currently only works on pull requests
on: [pull_request]

jobs:
build:
name: Lint ICU4C C++ executor
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

# Optionally generate compile_commands.json

# Run clang-tidy
# Note: when running locally at the command line, use the equivalent
# command when in the directory `executors/cpp`:
# clang-tidy *.cpp --fix-errors --config-file="clang-tidy-config.yml" -p .
# Note: you must run setup.sh and also run install_icu4c_binary.sh (for a given ICU4C version) first
# before running the above clang-tidy command
- uses: ZedThree/[email protected]
with:
# clang-tidy specific configs
build_dir: './executors/cpp'
config_file: './executors/cpp/clang-tidy-config.yml'
# Action-specific config
split_workflow: true
apt_packages: "libjson-c-dev,libicu-dev"
id: review

- uses: ZedThree/clang-tidy-review/[email protected]

# If there are any comments, fail the check
- if: steps.review.outputs.total_comments > 0
run: exit 1
2 changes: 1 addition & 1 deletion .github/workflows/e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ permissions:

jobs:
run_all:
name: End-to-end (Gen data, run tests, gen GH Pages)
name: End-to-end
runs-on: ubuntu-latest
steps:
- name: Checkout repo
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/run-rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
fail-fast: false
matrix:
icu4x-version: [ '1.3', '1.4' ]
name: Lint the executor code for ICU4X
name: Lint ICU4X Rust executor
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
Expand Down
1 change: 1 addition & 0 deletions executors/cpp/clang-tidy-config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
HeaderFilterRegex: '.*'
29 changes: 11 additions & 18 deletions executors/cpp/coll.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,10 @@ using icu::RuleBasedCollator;

const char error_message[] = "error";

bool debug = false;

/**
* TestCollator -- process JSON inputs, run comparator, return result
*/
const string TestCollator(json_object *json_in) {
string TestCollator(json_object *json_in) {
UErrorCode status = U_ZERO_ERROR;

json_object *label_obj = json_object_object_get(json_in, "label");
Expand All @@ -68,7 +66,7 @@ const string TestCollator(json_object *json_in) {

json_object *locale_obj = json_object_object_get(json_in, "locale");
const char *locale_string;
if (locale_obj) {
if (locale_obj != nullptr) {
locale_string = json_object_get_string(locale_obj);
} else {
locale_string = "und";
Expand All @@ -78,7 +76,7 @@ const string TestCollator(json_object *json_in) {
json_object *compare_type_obj =
json_object_object_get(json_in, "compare_type");
string compare_type_string = "";
if (compare_type_obj) {
if (compare_type_obj != nullptr) {
compare_type_string = json_object_get_string(compare_type_obj);
}

Expand All @@ -87,7 +85,7 @@ const string TestCollator(json_object *json_in) {
string strength_string = "";

json_object *strength_obj = json_object_object_get(json_in, "strength");
if (strength_obj) {
if (strength_obj != nullptr) {
strength_string = json_object_get_string(strength_obj);
if (strength_string == "primary") {
strength_type = Collator::PRIMARY;
Expand All @@ -105,14 +103,14 @@ const string TestCollator(json_object *json_in) {
// Check for rule-based collation
json_object *rules_obj = json_object_object_get(json_in, "rules");
string rules_string = "";
if (rules_obj) {
if (rules_obj != nullptr) {
rules_string = json_object_get_string(rules_obj);
}
UnicodeString uni_rules = UnicodeString::fromUTF8(rules_string);

// Allow for different levels or types of comparison.
json_object *compare_type = json_object_object_get(json_in, "compare_type");
if (compare_type) {
if (compare_type != nullptr) {
// TODO: Apply this in tests.
const char *comparison_type = json_object_get_string(compare_type);
}
Expand Down Expand Up @@ -172,12 +170,12 @@ const string TestCollator(json_object *json_in) {
return json_object_to_json_string(return_json);
}

if (strength_obj) {
if (strength_obj != nullptr) {
uni_coll->setStrength(strength_type);
}

if (ignore_obj) {
const bool ignore_punctuation_bool = json_object_get_boolean(ignore_obj);
if (ignore_obj != nullptr) {
const bool ignore_punctuation_bool = json_object_get_boolean(ignore_obj) != 0;
if (ignore_punctuation_bool) {
uni_coll->setAttribute(UCOL_ALTERNATE_HANDLING, UCOL_SHIFTED, status);
if (check_icu_error(
Expand All @@ -202,7 +200,7 @@ const string TestCollator(json_object *json_in) {
return json_object_to_json_string(return_json);
}

if (uni_coll) {
if (uni_coll != nullptr) {
uni_coll->getAttribute(UCOL_ALTERNATE_HANDLING, status); // ignore result
}
delete uni_coll;
Expand All @@ -214,11 +212,6 @@ const string TestCollator(json_object *json_in) {
coll_result = (uni_result != UCOL_GREATER);
if (!coll_result) {
// Test did not succeed!
if (debug) {
cout << "# UNI_RESULT: " << label_string << " " << uni_result <<
" s1: " << string1 << " s2: " << string2 << endl;
}

// Include data compared in the failing test
json_object_object_add(
return_json, "s1", json_object_new_string(string1.c_str()));
Expand All @@ -231,7 +224,7 @@ const string TestCollator(json_object *json_in) {
}

json_object_object_add(
return_json, "result", json_object_new_boolean(coll_result));
return_json, "result", json_object_new_boolean(static_cast<json_bool>(coll_result)));

return json_object_to_json_string(return_json);
}
9 changes: 9 additions & 0 deletions executors/cpp/compile_flags.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
-L/tmp/icu/icu/usr/local/lib
-I/usr/include/c++/13
-I/usr/include/x86_64-linux-gnu/c++/13
-I/usr/include/c++/13/backward
-I/usr/lib/gcc/x86_64-linux-gnu/13/include
-I/usr/local/include
-I/usr/include/x86_64-linux-gnu
-I/usr/include

50 changes: 27 additions & 23 deletions executors/cpp/datetime_fmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

#include <json-c/json.h>

#include <stdio.h>
#include <stdlib.h>
#include <cstdio>
#include <cstdlib>

#include <iostream>
#include <string>
Expand Down Expand Up @@ -33,15 +33,19 @@ using std::cout;
using std::endl;
using std::string;

icu::DateFormat::EStyle StringToEStyle(string style_string) {
if (style_string == "full") return icu::DateFormat::kFull;
if (style_string == "long") return icu::DateFormat::kLong;
if (style_string == "medium") return icu::DateFormat::kMedium;
if (style_string == "short") return icu::DateFormat::kShort;
auto StringToEStyle(string style_string) -> icu::DateFormat::EStyle {
if (style_string == "full") { return icu::DateFormat::kFull;
}
if (style_string == "long") { return icu::DateFormat::kLong;
}
if (style_string == "medium") { return icu::DateFormat::kMedium;
}
if (style_string == "short") { return icu::DateFormat::kShort;
}
return icu::DateFormat::kNone;
}

const string TestDatetimeFmt(json_object *json_in) {
string TestDatetimeFmt(json_object *json_in) {
UErrorCode status = U_ZERO_ERROR;

json_object *label_obj = json_object_object_get(json_in, "label");
Expand All @@ -55,7 +59,7 @@ const string TestDatetimeFmt(json_object *json_in) {
// The locale for formatted output
json_object *locale_label_obj = json_object_object_get(json_in, "locale");
string locale_string;
if (locale_label_obj) {
if (locale_label_obj != nullptr) {
locale_string = json_object_get_string(locale_label_obj);
} else {
locale_string = "und";
Expand All @@ -72,19 +76,19 @@ const string TestDatetimeFmt(json_object *json_in) {
// Get fields out of the options if present
json_object* options_obj = json_object_object_get(json_in, "options");

if (options_obj) {
if (options_obj != nullptr) {
// Check for timezone and calendar
json_object* option_item =
json_object_object_get(options_obj, "timeZone");
if (option_item) {
if (option_item != nullptr) {
string timezone_str = json_object_get_string(option_item);
UnicodeString u_tz(timezone_str.c_str());
tz = TimeZone::createTimeZone(u_tz);
}

json_object* cal_item =
json_object_object_get(options_obj, "calendar");
if (cal_item) {
if (cal_item != nullptr) {
calendar_str = json_object_get_string(cal_item);
}
}
Expand All @@ -93,12 +97,12 @@ const string TestDatetimeFmt(json_object *json_in) {
locale_string = locale_string + "@calendar=" + calendar_str;
display_locale = locale_string.c_str();

if (tz) {
if (tz != nullptr) {
cal = Calendar::createInstance(tz, display_locale, status);
} else {
cal = Calendar::createInstance(display_locale, status);
}
if (U_FAILURE(status)) {
if (U_FAILURE(status) != 0) {
json_object_object_add(
return_json,
"error",
Expand All @@ -125,15 +129,15 @@ const string TestDatetimeFmt(json_object *json_in) {
// skeleton or date_style.
string default_skeleton_string = "M/d/yyyy";

if (options_obj) {
if (options_obj != nullptr) {
json_object* option_item = json_object_object_get(options_obj, "dateStyle");
if (option_item) {
if (option_item != nullptr) {
dateStyle_str = json_object_get_string(option_item);
date_style = StringToEStyle(dateStyle_str);
}

option_item = json_object_object_get(options_obj, "timeStyle");
if (option_item) {
if (option_item != nullptr) {
timeStyle_str = json_object_get_string(option_item);
time_style = StringToEStyle(timeStyle_str);
}
Expand All @@ -146,13 +150,13 @@ const string TestDatetimeFmt(json_object *json_in) {
time_style == icu::DateFormat::EStyle::kNone) {
skeleton_string = default_skeleton_string;
}
if (date_skeleton_obj) {
if (date_skeleton_obj != nullptr) {
// Data specifies a date time skeleton. Make a formatter based on this.
skeleton_string = json_object_get_string(date_skeleton_obj);
}
if (skeleton_string != "") {
UnicodeString u_skeleton(skeleton_string.c_str());
if (cal) {
if (cal != nullptr) {
df = DateFormat::createInstanceForSkeleton(cal,
u_skeleton,
display_locale,
Expand Down Expand Up @@ -183,7 +187,7 @@ const string TestDatetimeFmt(json_object *json_in) {
}

// !!! IS OFFSET ALREADY CONSIDERED?
if (tz) {
if (tz != nullptr) {
df->setTimeZone(*tz);
}

Expand All @@ -194,7 +198,7 @@ const string TestDatetimeFmt(json_object *json_in) {
json_object *input_millis = json_object_object_get(json_in, "input_millis");

UDate test_date_time;
if (input_string_obj) {
if (input_string_obj != nullptr) {
Locale und_locale("und");

string input_date_string = json_object_get_string(input_string_obj);
Expand All @@ -219,7 +223,7 @@ const string TestDatetimeFmt(json_object *json_in) {

// TODO: handles the offset +/-
SimpleDateFormat iso_date_fmt(u"y-M-d'T'h:m:sZ", und_locale, status);
if (U_FAILURE(status)) {
if (U_FAILURE(status) != 0) {
string error_name = u_errorName(status);
string error_message =
"# iso_date_fmt constructor failure: " +
Expand All @@ -240,7 +244,7 @@ const string TestDatetimeFmt(json_object *json_in) {
"# iso_date_fmt parse failure" + input_date_string)) {
return json_object_to_json_string(return_json);
}
} else if (input_millis) {
} else if (input_millis != nullptr) {
test_date_time = json_object_get_double(input_millis);
} else {
json_object_object_add(
Expand Down
16 changes: 8 additions & 8 deletions executors/cpp/likely_subtags.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
#include <unicode/unistr.h>
#include <unicode/utypes.h>

#include <stdio.h>
#include <stdlib.h>
#include <cstdio>
#include <cstdlib>

#include <cstring>
#include <iostream>
Expand All @@ -25,7 +25,7 @@ using icu::Locale;
using icu::StringByteSink;
using icu::UnicodeString;

const string TestLikelySubtags(json_object *json_in) {
string TestLikelySubtags(json_object *json_in) {
UErrorCode status = U_ZERO_ERROR;

json_object *label_obj = json_object_object_get(json_in, "label");
Expand Down Expand Up @@ -55,12 +55,12 @@ const string TestLikelySubtags(json_object *json_in) {
Locale maximized(displayLocale);
maximized.addLikelySubtags(status);

if (U_FAILURE(status)) {
if (U_FAILURE(status) != 0) {
test_result = "error in maximize";
} else {
maximized.toLanguageTag(byteSink, status);

if (U_FAILURE(status)) {
if (U_FAILURE(status) != 0) {
json_object_object_add(
return_json,
"error",
Expand All @@ -72,14 +72,14 @@ const string TestLikelySubtags(json_object *json_in) {
option_string == "minimizeFavorRegion") {
// Minimize
displayLocale.minimizeSubtags(status);
if (U_FAILURE(status)) {
if (U_FAILURE(status) != 0) {
const string error_message_min = "error in minimize";
test_result = error_message_min;
} else {
displayLocale.toLanguageTag(byteSink, status);
test_result = name_string;

if (U_FAILURE(status)) {
if (U_FAILURE(status) != 0) {
json_object_object_add(
return_json,
"error",
Expand Down Expand Up @@ -120,7 +120,7 @@ const string TestLikelySubtags(json_object *json_in) {
json_object_new_string(option_string.c_str()));
}

if (U_FAILURE(status)) {
if (U_FAILURE(status) != 0) {
json_object_object_add(
return_json,
"error",
Expand Down
Loading
Loading