diff --git a/src/maketools/plumedcheck b/src/maketools/plumedcheck index f9ebaafd555831ff8ddc77a17cdff3c7b719230f..a6c94f835710ff8847e3f779f1517604d6793e61 100755 --- a/src/maketools/plumedcheck +++ b/src/maketools/plumedcheck @@ -1,4 +1,5 @@ #! /bin/bash +# vim: ft=awk TEMP=$(mktemp) @@ -99,6 +100,22 @@ BEGIN{ deprecated_headers["stdlib.h"]=1 deprecated_headers["string.h"]=1 deprecated_headers["tgmath.h"]=1 + +# list of allowed module types + allowed_module_types["always"]=1 + allowed_module_types["default-on"]=1 + allowed_module_types["default-off"]=1 + +# list of "outer modules", that is those +# that are not included in the kernel library + outer_modules["wrapper"]=1 + outer_modules["main"]=1 + +# list of "core modules", that is those for which +# a specific module namespace is not required + core_modules["core"]=1 + core_modules["tools"]=1 + core_modules["reference"]=1 } # for each input file @@ -131,6 +148,11 @@ BEGINFILE{ if((getline < path > 0) ){ module_type[module]=$0 information("module_type","module " module " has type '" module_type[module] "'" ); +# DOC: :wrong_module_type: +# DOC: Module type as indicated in the `module.type` file located in the module directory is not valid. +# DOC: The admitted types are: `default-on`, which means the module is on by default, `default-off`, which means that +# DOC: they should be explicitly enabled, and `always`, which means they are always required. + if(!(module_type[module] in allowed_module_types)) error("wrong_module_type","module " module " has a wrong module type '" module_type[module] "'"); } } } @@ -159,6 +181,12 @@ BEGINFILE{ # detect copyright lines if($0=="/* +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++") in_copyright=1; # check direct inclusion of headers that would bypass module dependencies +# DOC: :include_dots: +# DOC: It is considered an error to include a file using a path starting with `..`. +# DOC: The reason is that this would allow to include PLUMED modules that have not been +# DOC: explicitly declared in the Makefile (e.g. using `#include "../tools/Vector.h"`). +# DOC: This might result in undetected dependences between the modules, which means that +# DOC: by enabling some set of modules one would result in a non-linkable library. if(match($0,"^# *include +\\\"\\.\\.")) error("include_dots","cannot include files using .. path"); # check if there is anything behind copyright and preprocessor commands @@ -177,12 +205,39 @@ BEGINFILE{ # } # } -# check if deprecated headers are included - if(match($0,"^# *include +<[^>]+>.*$")){ +# check #include <file> + if(!in_doxygen && match($0,"^# *include +<[^>]+>.*$")){ h=$0 sub("^# *include +<","",h) sub(">.*$","",h) +# check if deprecated headers are included +# DOC: :deprecated_headers: +# DOC: There is a list of headers that have been deprecated in C++ and should not be +# DOC: included. These headers should be replaced with their equivalent from the stdlib. +# DOC: E.g., use `#include <cstdlib>` instead of `#include <stdlib.h>`. Notice that +# DOC: using the modern header all the functions are declared inside the `std` namespace. if(h in deprecated_headers) error("deprecated_header","including deprecated header " h); +# DOC: :external_header: (style) +# DOC: Header files should possibly not include other header files from external libraries. +# DOC: Indeed, once PLUMED is installed, if paths are not set correctly the include files of the other libraries +# DOC: might not be reachable anymore. This is only a stylistic warning now. Notice that +# DOC: with the current implementation of class Communicator it is necessary to include +# DOC: `mpi.h` in a header file. + if(filetype=="header" && h~"\\.h") style("external_header","including external header " h " in a header file"); + } + +# check #include "file" + if(!in_doxygen && match($0,"^# *include +\"[^\"]+\".*$")){ + h=$0 + sub("^# *include +\"","",h) + sub("\".*$","",h) +# check if a .inc file, which is temporary and not installed, is included in a header file. +# this might create inconsistencies in the installed plumed header files. +# DOC: :non_h_header: +# DOC: Files with `.inc` extension are generated by PLUMED while it is compiled and are +# DOC: not installed. They should not be directly included in header files, otherwise +# DOC: those header files could be not usable once PLUMED is installed. + if(filetype=="header" && h!~"\\.h") error("non_h_header","including non '.h' file " h " in a header file"); } # check if namespace PLMD is present @@ -197,7 +252,9 @@ BEGINFILE{ sub("^ *PLUMED_REGISTER_ACTION\\(.*, *[\"]","",action); sub("[\"].*$","",action); information("registered_action","action " action); - if(action in registered_actions) error("multi_registered","action " action " already registered at "registered_actions[action]); +# DOC: :multi_registered_action: +# DOC: Each action should be registered (with `PLUMED_REGISTER_ACTION`) once and only once. + if(action in registered_actions) error("multi_registered_action","action " action " already registered at "registered_actions[action]); registered_actions[action]=FILENAME ":" FNR; } @@ -207,16 +264,57 @@ BEGINFILE{ sub("^ *PLUMED_REGISTER_CLTOOL\\(.*, *[\"]","",cltool); sub("[\"].*$","",cltool); information("registered_cltool","cltool " cltool); - if(cltool in registered_cltools) error("multi_registered","cltool " cltool " already registered at "registered_cltools[cltool]); +# DOC: :multi_registered_cltool: +# DOC: Each cltool should be registered (with `PLUMED_REGISTER_CLTOOL`) once and only once. + if(cltool in registered_cltools) error("multi_registered_cltool","cltool " cltool " already registered at "registered_cltools[cltool]); registered_cltools[cltool]=FILENAME ":" FNR; } +# take note of registered vessels + if(match($0,"^ *PLUMED_REGISTER_VESSEL")){ + vessel=$0 + sub("^ *PLUMED_REGISTER_VESSEL\\(.*, *[\"]","",vessel); + sub("[\"].*$","",vessel); + information("registered_vessel","vessel " vessel); +# DOC: :multi_registered_vessel: +# DOC: Each vessel should be registered (with `PLUMED_REGISTER_VESSEL`) once and only once. + if(vessel in registered_vessels) error("multi_registered_vessel","vessel " vessel " already registered at "registered_vessels[vessel]); + registered_vessels[vessel]=FILENAME ":" FNR; + } + +# take note of registered metrics + if(match($0,"^ *PLUMED_REGISTER_METRIC")){ + metric=$0 + sub("^ *PLUMED_REGISTER_METRIC\\(.*, *[\"]","",metric); + sub("[\"].*$","",metric); + information("registered_metric","metric " metric); +# DOC: :multi_registered_metric: +# DOC: Each metric should be registered (with `PLUMED_REGISTER_METRIC`) once and only once. + if(metric in registered_metrics) error("multi_registered_metric","metric " metric " already registered at "registered_metrics[metric]); + registered_metrics[metric]=FILENAME ":" FNR; + } + # take note of documented actions or cltools if(match($0,"^//[+]PLUMEDOC ")){ doc=$NF; - information("documented","doc " doc); +# DOC: :multi_doc: +# DOC: An action or cltool should be documented once and only once. Notice that because of the +# DOC: way the manual is generated, a cltool cannot have the same name of an action. if(doc in plumed_doc) error("multi_doc","doc " doc " already at "plumed_doc[action]); plumed_doc[doc]=FILENAME ":" FNR; + switch($(NF-1)){ + case "TOOLS": + plumed_doc_cltools[doc]=FILENAME ":" FNR; + information("documented_cltool","doc " doc); + break; + case "INTERNAL": + plumed_doc_internal[doc]=FILENAME ":" FNR; + information("documented_internal","doc " doc); + break; + default: + plumed_doc_action[doc]=FILENAME ":" FNR; + information("documented_action","doc " doc); + } } # check if, besides the \par Examples text, there is a verbatim command @@ -235,6 +333,12 @@ BEGINFILE{ if(match($0,guard)) found_guard_ifdef=1 # check that "using namespace" statements are not used in header files +# DOC: :using_namespace_in_header: +# DOC: A statement `using namespace` in header files is considered bad practice. Indeed, this would imply that +# DOC: any other file including that header file would be using the same namespace. +# DOC: This is true also for `std` namespace, and that's why you typically find full specifications +# DOC: such as `std::string` in header files. In source files you are free to use `using namespace PLMD` and/or +# DOC: `using namespace std` if you like, since these commands will only affect the source file where they are used. if(filetype=="header" && match($0,"using *namespace") && !in_plumed_doc && !in_doxygen) error("using_namespace_in_header","using namespace statement in header file") } @@ -244,11 +348,20 @@ BEGINFILE{ sub("__PLUMED_HAS_","",string); if(filetype=="autoconf"){ # take note of defined HAS macros - information("defined_has",string); + if(plumed_definehas_file[string,FILENAME]!="used") information("defined_has",string); + plumed_definehas_file[string,FILENAME]="used"; plumed_definehas[string]=FILENAME ":" FNR; } else { # take note of used HAS macros - information("used_has",string); + if(plumed_usehas_file[string,FILENAME]!="used") information("used_has",string); +# DOC: :used_has_in_header: (style) +# DOC: Using `__PLUMED_HAS_SOMETHING` macros in headers should be avoided since +# DOC: it could make it difficult to make sure the same macros are correctly defined +# DOC: when using PLUMED as a library. This is only a stylistic warning now. Notice that +# DOC: with the current implementation of class Communicator it is necessary to include +# DOC: `mpi.h` in a header file and thus to use `__PLUMED_HAS_MPI`. + if(plumed_usehas_file[string,FILENAME]!="used" && filetype=="header") style("used_has_in_header",string); + plumed_usehas_file[string,FILENAME]="used"; plumed_usehas[string]=FILENAME ":" FNR; } } @@ -267,21 +380,28 @@ BEGINFILE{ # at the end of each file: ENDFILE{ # check for namespaces -# this is done only for non-trivial files (i.e. we skip files that just include an header) -# we also skip special modules wrapper and main here, which do not define PLMD namespace - if(found_non_preprocessor_lines && (filetype=="source" || filetype=="header") && module!="wrapper" && module!="main" ){ + if(found_non_preprocessor_lines && (filetype=="source" || filetype=="header") && !(module in outer_modules) ){ +# DOC: :missing_namespace_plmd: +# DOC: Every source file should contain a `PLMD` namespace. +# DOC: Notice that files in the "outer modules" `wrapper` and `main`, that is those that +# DOC: are not included in the kernel library, are exempted from this rule. if(!found_namespace_plmd) error("missing_namespace_plmd","missing PLMD namespace"); -# files belonging to "core modules" are not supposed to have a module namespace - if(module!="core" && module!="tools" && module!="reference"){ - # this can be done only if module name is known: - if(module && !found_namespace_module) error("missing_namespace_module","missing " module " namespace") - } +# DOC: :missing_namespace_module: +# DOC: Every source file should contain a sub namespace with the same name of the module itself. +# DOC: Notice that there are important exceptions to this rule: +# DOC: - "outer modules" `wrapper` and `main`, that are not included in the kernel library. +# DOC: - "core modules" that are part of the core of PLUMED. Since the classes implemented here are +# DOC: widely used within PLUMED, they are directly defined in the PLMD namespace. + if(module && !(module in core_modules) && !found_namespace_module) error("missing_namespace_module","missing " module " namespace") } -# check if header guard is present -# check is done only if module is known since module name is used to build the macro name if(filetype=="header"){ - # this can be done only if module name is known: +# this can be done only if module name is known: +# DOC: :missing_guard: +# DOC: Every header file should contain a proper guard to avoid double inclusion. +# DOC: The format of the guard is fixed and should be `__PLUMED_modulename_filename_h`, where +# DOC: `modulename` is the name of the module and `filename` is the name of the file, without suffix. +# DOC: This choice makes guards unique within the whole codebase. if(module && !found_guard_ifdef) error("missing_guard","missing ifndef guard"); } @@ -297,11 +417,18 @@ ENDFILE{ if((getline < cppfile < 0)) notfound=1 } if(notfound){ +# DOC: :non_existing_cpp: +# DOC: For every header file there should exist a corresponding source file with the same name. +# DOC: Notice that dummy headers that only include another header or which only define preprocessor +# DOC: macros are exempted from this rule. error("non_existing_cpp","file " file " is a header but there is no corresponding source"); } while((getline < cppfile)>=0){ if(match($0,"^ *#include")){ sub("^ *#include",""); +# DOC: :non_included_h: +# DOC: The source file corresponding to a header file (that is: with the same name) should include it as the first included file. +# DOC: This is to make sure that all the header files that we install can be individually included and compiled. if($1!="\"" file "\"") error("non_included_h","file " file " is a header but " cppfile " does not include it as first include"); break; } @@ -318,39 +445,71 @@ END{ # this is required to properly report messages global_check=1 -# check that all registered actions were documented for(action in registered_actions){ - if(!(action in plumed_doc)){ + if(!(action in plumed_doc_action)){ +# DOC: :undocumented_action: +# DOC: Every action that is registered with `PLUMED_REGISTER_ACTION` should also be documented +# DOC: in a PLUMEDOC page. error("undocumented_action","action " action " at " registered_actions[action] " is not documented") } else if(!(action in provide_examples)){ - error("action_without_examples","action " action " at " plumed_doc[action] " has no example") +# DOC: :action_without_examples: +# DOC: Every action that is registered should contain an `Example` section in its documentation. +# DOC: Notice that if the `Example` section is not added the manual will not show the registered keywords. + error("action_without_examples","action " action " at " plumed_doc_action[action] " has no example") } else if(!(action in provide_verbatim)){ -# this should be made into an error: +# DOC: :action_without_verbatim: +# DOC: Every action that is registered should contain an example in the form of a `verbatim` section. +# DOC: This is currently a stylistic warning since there are many actions not satisfying this requirement. style("action_without_verbatim","action " action " at " provide_examples[action] " has no verbatim") } } -# check that all registered cltools were documented + for(action in plumed_doc_action){ + if(!(action in registered_actions)) +# DOC: :unregistered_action: +# DOC: Every action that is documented should be also registered using `PLUMED_REGISTER_ACTION`. + error("unregistered_action"," action " action " at " plumed_doc_action[action] " is not registered"); + } + for(cltool in registered_cltools){ - if(!(cltool in plumed_doc)){ + if(!(cltool in plumed_doc_cltools)){ +# DOC: :undocumented_cltool: +# DOC: Every cltool that is registered with `PLUMED_REGISTER_CLTOOL` should also be documented +# DOC: in a PLUMEDOC page. error("undocumented_cltool","cltool " cltool " at " registered_cltools[cltool] " is not documented") } else if(!(cltool in provide_examples)){ - error("cltool_without_examples","cltool " cltool " at " plumed_doc[cltool] " has no example") +# DOC: :cltool_without_examples: +# DOC: Every cltool that is registered should contain an `Example` section in its documentation. +# DOC: Notice that if the `Example` section is not added the manual will not show the registered options. + error("cltool_without_examples","cltool " cltool " at " plumed_doc_cltools[cltool] " has no example") } else if(!(cltool in provide_verbatim)){ +# DOC: :cltool_without_verbatim: +# DOC: Every cltool that is registered should contain an example in the form of a `verbatim` section. error("cltool_without_verbatim","cltool " cltool " at " provide_examples[cltool] " has no verbatim") } } -# check that all used __PLUMED_HAS macros have been defined in autoconf + for(cltool in plumed_doc_cltools){ + if(!(cltool in registered_cltools)) +# DOC: :unregistered_cltool: +# DOC: Every cltool that is documented should be also registered using `PLUMED_REGISTER_CLTOOL`. + error("unregistered_cltool"," cltool " cltool " at " plumed_doc_cltools[cltool] " is not registered"); + } + for(has in plumed_usehas){ if(!(has in plumed_definehas)){ +# DOC: :undefined_has: +# DOC: Every macro in the form `__PLUMED_HAS_SOMETHING` that is used in the code +# DOC: should be defined in configure.ac. This check is made to avoid errors with mis-typed macros. error("undefined_has","has " has " at " plumed_usehas[has] " is not defined") } } -# check that all defined __PLUMED_HAS macros have been used in source code for(has in plumed_definehas){ if(!(has in plumed_usehas)){ +# DOC: :unused_has: +# DOC: Every macro in the form `__PLUMED_HAS_SOMETHING` that is defined in configure.ac +# DOC: should be used at least once in the code. This check is made to avoid errors with mis-typed macros. error("unused_has","has " has " at " plumed_definehas[has] " is not used") } }