jeff-hykin / better-cpp-syntax

💾 The source of VS Code's C++ syntax highlighting
GNU General Public License v3.0
154 stars 29 forks source link

Detect bad include guard #374

Open matter123 opened 5 years ago

matter123 commented 5 years ago

Include guards are in the form of:

#ifndef INCLUDE_GUARD_NAME
#define INCLUDE_GUARD_NAME
...
#endif

It may be possible using a while Pattern Range to detect when the define does not match the ifndef

Something like:

PatterRange.new(
    start_pattern: Pattern.new(/#ifndef/).then(std_space).then(
        match: /\w++/,
        reference: "include_guard_name",
    ).then(std_space).then(@end_of_line),
    while_pattern: @start_of_line.then(std_space).then(/#define/).then(std_space).oneOf([
        Pattern.new(
            match: matchResultOf("include_guard_name"),
            tag_as: "entity.name.function.preprocessor",
        ).then(std_space),
        Pattern.new(
            match: /\w++/,
            tag_as: "invalid...include_guard...",
        )
    ]).then(std_space).then(@end_of_line),
)

Research should probably be done to determine the incidences of false positives. Hopefully, false positives are minimized due to the requirement that the defined macro has no value.

This does mark SOME_DEFINE below as bad, but I think that can be fixed with another PatternRange (A begin/end that ends at @end_of_line except when \G would match, this only allows for the while pattern to continue for one line)

#ifndef HEADER_NAME
#define HEADER_NAME
#define SOME_DEFINE
...
#endif
matter123 commented 5 years ago

After testing on wxWidgets, llvm, qt, and boost, I found very little false positives, and those were eliminated when, I limited the include guard detection to the first detected.

Edit: It did detect one case of a typo in the include guard ./boost/libs/fusion/include/boost/fusion/include/adapt_assoc_adt.hpp:

#ifndef BOOST_FUSION_INCLUDE_ADAPT_ASSOC_ADT_HPP
#define BOOST_FUSION_INCLUDE_ADAPT_ASSOC_ADR_HPP

#include <boost/fusion/support/config.hpp>
#include <boost/fusion/adapted/adt/adapt_assoc_adt.hpp>

#endif

If you want to test yourself I used the following script

include_guard_name = nil

File.readlines(ARGV[0]).each do |line|
    if /^#ifndef\s+(\w+)\s*$/ =~ line
        include_guard_name = $1
    elsif /^#define\s+(\w+)\s*$/ =~ line
        next if include_guard_name.nil?

        if $1 != include_guard_name
            puts ARGV[0]
            puts include_guard_name, $1
            puts ""
        end
        # simulate only the first #ifndef/#define checking for include guard
        exit
    else
        include_guard_name = nil
    end
end
matter123 commented 5 years ago

Of my more standard random c++ projects to test (boost, chromium, firefox-68, gcc, libreoffice, llvm):

The unexpected pattern in source files is actually the easiest to fix.

The 4 can be avoided by makes sure that pattern only works when in the root context.

That leaves 2 genuine errors that are not easily fixed.