Skip to content

Add AStyle definition file and travis astyle check #6014

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

Closed
wants to merge 22 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
  •  
  •  
  •  
17 changes: 17 additions & 0 deletions .astyleignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
BUILD
cmsis
features/mbedtls
features/FEATURE_LWIP/lwip
rtos/TARGET_CORTEX/rtx4
features/filesystem/littlefs/littlefs
features/filesystem/fat/ChaN
features/frameworks
features/FEATURE_BLE/targets
features/FEATURE_LWIP/lwip-interface/lwip
features/unsupported/
features/FEATURE_COMMON_PAL/
FEATURE_NANOSTACK/coap-service
FEATURE_NANOSTACK/sal-stack-nanostack
rtos/TARGET_CORTEX/rtx5
targets
tools
34 changes: 34 additions & 0 deletions .astylerc
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Mbed OS code style definition file for astyle

# Don't create backup files, let git handle it
suffix=none

# K&R style
style=kr

# 1 TBS addition to k&r, add braces to one liners
# Use -j as it was changed in astyle from brackets to braces, this way it is compatible with older astyle versions
-j

# 4 spaces, convert tabs to spaces
indent=spaces=4
convert-tabs

# Indent switches and cases
indent-switches
indent-cases

# Remove spaces in and around parentheses
unpad-paren

# Insert a space after if, while, for, and around operators
pad-header
pad-oper

# Pointer/reference operators go next to the name (on the right)
align-pointer=name
align-reference=name

# Attach { for classes and namespaces
attach-namespaces
attach-classes
35 changes: 35 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,41 @@ matrix:
# Report success since we have overridden default behaviour
- bash -c "$STATUS" success "Local $NAME testing has passed"

- env:
- NAME=astyle
install:
- wget https://downloads.sourceforge.net/project/astyle/astyle/astyle%203.1/astyle_3.1_linux.tar.gz;
mkdir -p BUILD && tar xf astyle_3.1_linux.tar.gz -C BUILD;
pushd BUILD/astyle/build/gcc;
make;
export PATH=$PWD/bin:$PATH;
popd;
- astyle --version
script:
- find -regex '.*\.\(h\|c\|hpp\|cpp\)' -type f | fgrep -v -f .astyleignore | xargs -n 100 -I {} bash -c "astyle -n --options=.astylerc \"{}\"" > astyle.out;
if [ $(cat astyle.out | grep Formatted | wc -l) -ne 0 ]; then
git --no-pager diff;
echo "Please fix style issues as shown above";
else
echo "Coding style check OK";
fi
after_success:
# update status if we succeeded, compare with master if possible
- |
CURR=$(cat astyle.out | grep Formatted | wc -l)
PREV=$(curl https://github.com/api/repos/$TRAVIS_REPO_SLUG/status/master \
| jq -re "select(.sha != \"$TRAVIS_COMMIT\")
| .statuses[] | select(.context == \"travis-ci/$NAME\").description
| capture(\", (?<warnings>[0-9]+) warnings\").warnings" \
|| echo 0)

STATUSM="Passed, ${CURR} warnings"
if [ "$PREV" -ne 0 ]
then
STATUSM="$STATUSM ($(python -c "print '%+d' % ($CURR-$PREV)") warnings)"
fi
- bash -c "$STATUS" success "$STATUSM"

- env:
- NAME=events
- EVENTS=events
Expand Down
81 changes: 52 additions & 29 deletions TESTS/events/queue/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,32 +31,38 @@ using namespace utest::v1;
volatile bool touched = false;

// static functions
void func5(int a0, int a1, int a2, int a3, int a4) {
void func5(int a0, int a1, int a2, int a3, int a4)
{
touched = true;
TEST_ASSERT_EQUAL(a0 | a1 | a2 | a3 | a4, 0x1f);
}

void func4(int a0, int a1, int a2, int a3) {
void func4(int a0, int a1, int a2, int a3)
{
touched = true;
TEST_ASSERT_EQUAL(a0 | a1 | a2 | a3, 0xf);
TEST_ASSERT_EQUAL(a0 | a1 | a2 | a3, 0xf);
}

void func3(int a0, int a1, int a2) {
void func3(int a0, int a1, int a2)
{
touched = true;
TEST_ASSERT_EQUAL(a0 | a1 | a2, 0x7);
}

void func2(int a0, int a1) {
void func2(int a0, int a1)
{
touched = true;
TEST_ASSERT_EQUAL(a0 | a1, 0x3);
}

void func1(int a0) {
void func1(int a0)
{
Copy link
Contributor

@geky geky Feb 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was curious about this so I tried astyle with both brace styles:

with style=kr:      1841070 line changes
with style=attach:  1819522 line changes

It would actually take fewer code changes to change the existing KR braces than to standardize on KR. We should reconsider our stance on brace placement.

I've never understood why our brace style is inconsistent between functions and every other syntactic construct.

Copy link
Contributor Author

@0xc0170 0xc0170 Feb 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would actually take fewer code changes to change the existing KR braces than to standardize on KR. We should reconsider our stance on brace placement.

I have testing online compiler - we need to match what we have here and what online compiler does. It seems it defines a rule to actually attach { in the header files, will run few more tests and get its options that it defines (only for C++ functions), AStyle has this option (attach-inlines) that seems to be active in the online compiler

The line 59 is correct, what we need to align is C++ inlines. From experience, not always it was clear when to attach { for users thus our initial coding style set it always new line for functions (no matter if C++ or C). This however as we can see it is not aligned with online compiler, will check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this PR, attached C++ as the online compiler does.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This contradict the guidelines and guidelines are made to be followed; the purpose of this PR is to enforce them. If the online compiler is wrong; fix the online compiler. Not the other way around.

Copy link
Contributor

@geky geky Feb 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KR style braces are still the minority, although only slightly:

with style=kr:     1819759 line changes
with style=attach: 1819522 line changes

I'm suggesting we change the guidelines to match the style of code users are contributing. I had no idea C++ inlines are a separate style, and if we don't know what our rules are on braces how are our users supposed to?

I mean what is this:

namespace hi {

class Thing {
    int dothing() {
        if (thing) {
            blahblahblah;
        } else {
            blahblahblah;
        }
    }
};

int dothing()
{
    while (thing) {
        blahblahblah;
    }
}

}

What's the reason for not making squiggly brackets consistent?

touched = true;
TEST_ASSERT_EQUAL(a0, 0x1);
}

void func0() {
void func0()
{
touched = true;
}

Expand Down Expand Up @@ -88,40 +94,44 @@ SIMPLE_POSTS_TEST(1, 0x01)
SIMPLE_POSTS_TEST(0)


void time_func(Timer *t, int ms) {
void time_func(Timer *t, int ms)
{
TEST_ASSERT_INT_WITHIN(5, ms, t->read_ms());
t->reset();
}

template <int N>
void call_in_test() {
void call_in_test()
{
Timer tickers[N];

EventQueue queue(TEST_EQUEUE_SIZE);

for (int i = 0; i < N; i++) {
tickers[i].start();
queue.call_in((i+1)*100, time_func, &tickers[i], (i+1)*100);
queue.call_in((i + 1) * 100, time_func, &tickers[i], (i + 1) * 100);
}

queue.dispatch(N*100);
queue.dispatch(N * 100);
}

template <int N>
void call_every_test() {
void call_every_test()
{
Timer tickers[N];

EventQueue queue(TEST_EQUEUE_SIZE);

for (int i = 0; i < N; i++) {
tickers[i].start();
queue.call_every((i+1)*100, time_func, &tickers[i], (i+1)*100);
queue.call_every((i + 1) * 100, time_func, &tickers[i], (i + 1) * 100);
}

queue.dispatch(N*100);
queue.dispatch(N * 100);
}

void allocate_failure_test() {
void allocate_failure_test()
{
EventQueue queue(TEST_EQUEUE_SIZE);
int id;

Expand All @@ -132,12 +142,14 @@ void allocate_failure_test() {
TEST_ASSERT(!id);
}

void no() {
void no()
{
TEST_ASSERT(false);
}

template <int N>
void cancel_test1() {
void cancel_test1()
{
EventQueue queue(TEST_EQUEUE_SIZE);

int ids[N];
Expand All @@ -146,7 +158,7 @@ void cancel_test1() {
ids[i] = queue.call_in(1000, no);
}

for (int i = N-1; i >= 0; i--) {
for (int i = N - 1; i >= 0; i--) {
queue.cancel(ids[i]);
}

Expand All @@ -157,31 +169,38 @@ void cancel_test1() {
// Testing the dynamic arguments to the event class
unsigned counter = 0;

void count5(unsigned a0, unsigned a1, unsigned a2, unsigned a3, unsigned a5) {
void count5(unsigned a0, unsigned a1, unsigned a2, unsigned a3, unsigned a5)
{
counter += a0 + a1 + a2 + a3 + a5;
}

void count4(unsigned a0, unsigned a1, unsigned a2, unsigned a3) {
void count4(unsigned a0, unsigned a1, unsigned a2, unsigned a3)
{
counter += a0 + a1 + a2 + a3;
}

void count3(unsigned a0, unsigned a1, unsigned a2) {
void count3(unsigned a0, unsigned a1, unsigned a2)
{
counter += a0 + a1 + a2;
}

void count2(unsigned a0, unsigned a1) {
void count2(unsigned a0, unsigned a1)
{
counter += a0 + a1;
}

void count1(unsigned a0) {
void count1(unsigned a0)
{
counter += a0;
}

void count0() {
void count0()
{
counter += 0;
}

void event_class_test() {
void event_class_test()
{
counter = 0;
EventQueue queue(TEST_EQUEUE_SIZE);

Expand All @@ -204,7 +223,8 @@ void event_class_test() {
TEST_ASSERT_EQUAL(counter, 30);
}

void event_class_helper_test() {
void event_class_helper_test()
{
counter = 0;
EventQueue queue(TEST_EQUEUE_SIZE);

Expand All @@ -227,7 +247,8 @@ void event_class_helper_test() {
TEST_ASSERT_EQUAL(counter, 15);
}

void event_inference_test() {
void event_inference_test()
{
counter = 0;
EventQueue queue(TEST_EQUEUE_SIZE);

Expand All @@ -252,7 +273,8 @@ void event_inference_test() {


// Test setup
utest::v1::status_t test_setup(const size_t number_of_cases) {
utest::v1::status_t test_setup(const size_t number_of_cases)
{
GREENTEA_SETUP(20, "default_auto");
return verbose_test_setup_handler(number_of_cases);
}
Expand All @@ -278,7 +300,8 @@ const Case cases[] = {

Specification specification(test_setup, cases);

int main() {
int main()
{
return !Harness::run(specification);
}

Loading