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

Added linters: cppcheck and cpplint #73

Merged
merged 10 commits into from
May 21, 2020
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
7 changes: 5 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@ env:
- BUILD_TYPE=RelWithDebInfo
script:
- mkdir build
- cd build
- cmake -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DCMAKE_CXX_FLAGS=-Werror ..
- cd build
- cmake -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DCMAKE_CXX_FLAGS=-Werror -DBUILD_TESTING=True ..
- make
- make test ARGS="-VV"
before_install:
- sudo apt-get update
- sudo apt-get install python3 cppcheck
after_script:
- sudo make install
- pkg-config --modversion console_bridge
8 changes: 5 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ generate_export_header(${PROJECT_NAME}
EXPORT_MACRO_NAME CONSOLE_BRIDGE_DLLAPI)

install(TARGETS ${PROJECT_NAME}
EXPORT ${PROJECT_NAME}-targets
EXPORT ${PROJECT_NAME}-targets
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
Expand Down Expand Up @@ -117,5 +117,7 @@ SET_DIRECTORY_PROPERTIES(PROPERTIES
ADDITIONAL_MAKE_CLEAN_FILES ${CMAKE_BINARY_DIR}/console_bridge-config.cmake
ADDITIONAL_MAKE_CLEAN_FILES ${CMAKE_BINARY_DIR}/console_bridge.pc)

enable_testing()
add_subdirectory(test)
if(BUILD_TESTING)
enable_testing()
add_subdirectory(test)
endif()
2 changes: 1 addition & 1 deletion QUALITY_DECLARATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ The copyright holders each provide a statement of copyright in each source code

### Linters and Static Analysis [4.v]

`libconsole-bridge-dev` is not being tested with linters or static analysis tools.
`libconsole-bridge-dev` is being tested with `cppcheck` and `cpplint`.

## Dependencies [5]

Expand Down
10 changes: 8 additions & 2 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,17 @@ configuration:
build_script:
- md build
- cd build
- cmake .. -DEXTRA_CMAKE_CXX_FLAGS="-WX"
- cmake .. -DEXTRA_CMAKE_CXX_FLAGS="-WX" -DBUILD_TESTING=True
- cmake --build . --config %CONFIGURATION%

install:
# Prepend newly installed Python to the PATH of this build (this cannot be
# done from inside the powershell script as it would require to restart
# the parent CMD process).
- "SET PATH=C:\\Python35;C:\\Python35\\Scripts;%PATH%"

test_script:
- cmake --build . --config %CONFIGURATION% --target RUN_TESTS

after_build:
- cmake --build . --config %CONFIGURATION% --target INSTALL
- cmake --build . --config %CONFIGURATION% --target INSTALL
50 changes: 27 additions & 23 deletions include/console_bridge/console.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@

/* Author: Ryan Luna, Ioan Sucan */

#ifndef CONSOLE_BRIDGE_CONSOLE_
#define CONSOLE_BRIDGE_CONSOLE_
#ifndef INCLUDE_CONSOLE_BRIDGE_CONSOLE_H_
#define INCLUDE_CONSOLE_BRIDGE_CONSOLE_H_

#include <string>

#include "console_bridge_export.h"
#include "./console_bridge_export.h"
ahcorde marked this conversation as resolved.
Show resolved Hide resolved

/** \file console.h
\defgroup logging Logging Macros
Expand Down Expand Up @@ -93,23 +93,22 @@ enum CONSOLE_BRIDGE_DLLAPI LogLevel

/** \brief Generic class to handle output from a piece of
code.

In order to handle output from the library in different
ways, an implementation of this class needs to be
provided. This instance can be set with the useOutputHandler
function. */
class CONSOLE_BRIDGE_DLLAPI OutputHandler
{
public:

OutputHandler(void)
{
}

virtual ~OutputHandler(void)
{
}

/** \brief log a message to the output handler with the given text
and logging level from a specific file and line number */
virtual void log(const std::string &text, LogLevel level, const char *filename, int line) = 0;
Expand All @@ -120,44 +119,45 @@ class CONSOLE_BRIDGE_DLLAPI OutputHandler
class CONSOLE_BRIDGE_DLLAPI OutputHandlerSTD : public OutputHandler
{
public:

OutputHandlerSTD(void) : OutputHandler()
{
}

virtual void log(const std::string &text, LogLevel level, const char *filename, int line);

};

/** \brief Implementation of OutputHandler that saves messages in a file. */
class CONSOLE_BRIDGE_DLLAPI OutputHandlerFile : public OutputHandler
{
public:

/** \brief The name of the file in which to save the message data */
OutputHandlerFile(const char *filename);
explicit OutputHandlerFile(const char *filename);

virtual ~OutputHandlerFile(void);

virtual void log(const std::string &text, LogLevel level, const char *filename, int line);

private:

/** \brief The file to save to */
FILE *file_;

};

/** \brief This function instructs ompl that no messages should be outputted. Equivalent to useOutputHandler(NULL) */
/** \brief This function instructs ompl that no messages should be outputted.
* Equivalent to useOutputHandler(NULL)
*/
CONSOLE_BRIDGE_DLLAPI void noOutputHandler(void);

/** \brief Restore the output handler that was previously in use (if any) */
CONSOLE_BRIDGE_DLLAPI void restorePreviousOutputHandler(void);

/** \brief Specify the instance of the OutputHandler to use. By default, this is OutputHandlerSTD */
/** \brief Specify the instance of the OutputHandler to use.
* By default, this is OutputHandlerSTD
*/
CONSOLE_BRIDGE_DLLAPI void useOutputHandler(OutputHandler *oh);

/** \brief Get the instance of the OutputHandler currently used. This is NULL in case there is no output handler. */
/** \brief Get the instance of the OutputHandler currently used.
* This is NULL in case there is no output handler.
*/
CONSOLE_BRIDGE_DLLAPI OutputHandler* getOutputHandler(void);

/** \brief Set the minimum level of logging data to output. Messages
Expand All @@ -171,8 +171,12 @@ CONSOLE_BRIDGE_DLLAPI LogLevel getLogLevel(void);
/** \brief Root level logging function. This should not be invoked directly,
but rather used via a \ref logging "logging macro". Formats the message
string given the arguments and forwards the string to the output handler */
CONSOLE_BRIDGE_DLLAPI void log(const char *file, int line, LogLevel level, const char* m, ...);
}
CONSOLE_BRIDGE_DLLAPI void log(const char *file,
int line,
LogLevel level,
const char* m,
...);
} // namespace console_bridge


#endif
#endif // INCLUDE_CONSOLE_BRIDGE_CONSOLE_H_
19 changes: 13 additions & 6 deletions src/console.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@

#include <iostream>
#include <mutex>
#include <string>
#include <utility>

/// @cond IGNORE

Expand All @@ -57,7 +59,8 @@ struct DefaultOutputHandler
console_bridge::OutputHandler *output_handler_;
console_bridge::OutputHandler *previous_output_handler_;
console_bridge::LogLevel logLevel_;
std::mutex lock_; // it is likely the outputhandler does some I/O, so we serialize it
// it is likely the outputhandler does some I/O, so we serialize it
std::mutex lock_;
};

// we use this function because we want to handle static initialization correctly
Expand Down Expand Up @@ -137,16 +140,17 @@ console_bridge::LogLevel console_bridge::getLogLevel(void)

static const char* LogLevelString[4] = {"Debug: ", "Info: ", "Warning: ", "Error: "};

void console_bridge::OutputHandlerSTD::log(const std::string &text, LogLevel level, const char *filename, int line)
void console_bridge::OutputHandlerSTD::log(const std::string &text,
LogLevel level,
const char *filename,
int line)
{
if (level >= CONSOLE_BRIDGE_LOG_WARN)
{
std::cerr << LogLevelString[level] << text << std::endl;
std::cerr << " at line " << line << " in " << filename << std::endl;
std::cerr.flush();
}
else
{
} else {
std::cout << LogLevelString[level] << text << std::endl;
std::cout.flush();
}
Expand All @@ -171,7 +175,10 @@ console_bridge::OutputHandlerFile::~OutputHandlerFile(void)
std::cerr << "Error closing logfile" << std::endl;
}

void console_bridge::OutputHandlerFile::log(const std::string &text, LogLevel level, const char *filename, int line)
void console_bridge::OutputHandlerFile::log(const std::string &text,
LogLevel level,
const char *filename,
int line)
{
if (file_)
{
Expand Down
41 changes: 41 additions & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,44 @@ foreach(GTEST_SOURCE_file ${tests})

set_tests_properties(${BINARY_NAME} PROPERTIES TIMEOUT 240)
endforeach()

find_package(PythonInterp 3 REQUIRED)

if(NOT PYTHONINTERP_FOUND)
message(STATUS "No PythonInterp found. Linters will not be executed")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think at the STATUS level, these messages will not be shown during a colcon build, though they would be at a WARNING level. I think that is fine, but I wanted to confirm this is the style of verbosity that is preferred

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed c8efe01

return()
endif()

find_program(WGET_EXE wget)
if(WGET_EXE)
message(STATUS "Found WGet: ${WGET_EXE}")
add_custom_target(wget_cppcheck
COMMAND ${WGET_EXE} -q -O cppcheck.py https://raw.githubusercontent.com/ament/ament_lint/master/ament_cppcheck/ament_cppcheck/main.py
ahcorde marked this conversation as resolved.
Show resolved Hide resolved
)

add_test(NAME console_bridge_cppcheck
COMMAND "${PYTHON_EXECUTABLE}"
"cppcheck.py"
"--language=c++" "${PROJECT_SOURCE_DIR}/src" "${PROJECT_SOURCE_DIR}/include"
)

add_custom_target(wget_cpplint
COMMAND wget -q -O cpplint.py https://raw.githubusercontent.com/ament/ament_lint/master/ament_cpplint/ament_cpplint/cpplint.py
)

add_test(NAME console_bridge_cpplint
COMMAND "${PYTHON_EXECUTABLE}"
"cpplint.py"
"--counting=detailed"
"--extensions=cpp,h"
"--linelength=100"
"--filter=-build/c++11,-runtime/references,-whitespace/braces,-whitespace/indent,-whitespace/parens,-whitespace/semicolon"
"${PROJECT_SOURCE_DIR}/include/console_bridge/console.h"
"${PROJECT_SOURCE_DIR}/src/console.cpp"
)

add_dependencies(console_bridge wget_cppcheck wget_cpplint)
ahcorde marked this conversation as resolved.
Show resolved Hide resolved

else()
message(STATUS "wget not found. Linters will not be executed")
endif()