Skip to content

Fix #671: Missing newline at end of file should produce a warning #672

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.

Sign up for GitHub

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

Open
glankk wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix #671: Missing newline at end of file should produce a warning #672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5252f99
Add warning for no newline at end of file
glankk Jul 8, 2026
f00b2eb
Rename to PORTABILITY_NO_EOF_NEWLINE and add case in main.cpp
glankk Jul 8, 2026
94f5500
Fix integration tests
glankk Jul 8, 2026
3b6ed82
Suppress PORTABILITY_NO_EOF_NEWLINE when standard is c++ or unknown
glankk Jul 14, 2026
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
24 changes: 12 additions & 12 deletions integration_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def __test_relative_header_create_header(dir, with_pragma_once=True):
#error header_was_already_included
#endif
const int dummy = 1;
""")
"""'\n')
return header_file, "error: #error header_was_already_included"

def __test_relative_header_create_source(dir, include1, include2, is_include1_sys=False, is_include2_sys=False, inv=False):
Expand All @@ -31,7 +31,7 @@ def __test_relative_header_create_source(dir, include1, include2, is_include1_sy
#undef TEST_H_INCLUDED
#include {format_include(include1, is_include1_sys)}
#include {format_include(include2, is_include2_sys)}
""")
"""'\n')
return src_file

@pytest.mark.parametrize("with_pragma_once", (False, True))
Expand Down Expand Up @@ -201,27 +201,27 @@ def test_same_name_header(record_property, tmpdir):
#include <header_a.h>
#include <header_b.h>
TEST
""")
"""'\n')

with open(header_a, "wt") as f:
f.write("""
#include "same_name.h"
""")
"""'\n')

with open(header_b, "wt") as f:
f.write("""
#include "same_name.h"
""")
"""'\n')

with open(same_name_a, "wt") as f:
f.write("""
#define TEST E
""")
"""'\n')

with open(same_name_b, "wt") as f:
f.write("""
#define TEST OK
""")
"""'\n')

args = [
format_include_path_arg(include_a),
Expand Down Expand Up @@ -279,13 +279,13 @@ def test_pragma_once_matching(record_property, tmpdir):
for n in names_to_test:
f.write(f"""
#include {n}
""");
"""'\n');

with open(once_header, "wt") as f:
f.write(f"""
#pragma once
ONCE
""");
"""'\n');

args = [
format_include_path_arg(test_dir),
Expand Down Expand Up @@ -463,7 +463,7 @@ def test_include_header_twice(tmpdir):
#ifdef BBB
# error BBB is defined
#endif
""")
"""'\n')

test_file = os.path.join(tmpdir, 'test.c')
with open(test_file, 'wt') as f:
Expand All @@ -473,7 +473,7 @@ def test_include_header_twice(tmpdir):

# define BBB
# include "test.h"
""")
"""'\n')

args = [test_file]

Expand Down Expand Up @@ -507,7 +507,7 @@ def test_define(record_property, tmpdir): # #589
def test_utf16_bom(tmpdir):
test_file = os.path.join(tmpdir, "test.cpp")
with open(test_file, 'wb') as f:
f.write(b'\xFF\xFE\x3B\x00')
f.write(b'\xFF\xFE\x3B\x00\x0A\x00')

args = [test_file]

Expand Down
8 changes: 8 additions & 0 deletions main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,14 @@ int main(int argc, char **argv)
case simplecpp::Output::PORTABILITY_BACKSLASH:
std::cerr << "portability: ";
break;
case simplecpp::Output::PORTABILITY_NO_EOF_NEWLINE:
if (simplecpp::getCStd(dui.std) == simplecpp::CUnknown) {
// Only UB for c code, suppress for c++ code
// If no standard is specified then prefer to have a false negative
continue;
}
std::cerr << "portability: ";

@danmar danmar Jul 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we could adjust main.cpp so that this output is only written if the filename extension is ".c".

@glankk glankk Jul 14, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think this is a bit of a clumsy way to solve this issue, I know there exists production code where c++ files have a .c extension (gdb is one example of this). We could have command-line flag for choosing the standard to use for simplecpp, but I think it might be better to just suppress this in the simplecpp executable and let cppcheck choose how to handle this when thrown from the simplecpp library. As far as I know neither the gcc or clang preprocessor have a warning for this, so there's precedence.

@glankk glankk Jul 14, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, I just realized we do have a -std flag for simplecpp. We should probably use that to decide what to do with this warning, and use the filename as a backup for autodetecting the standard to use.

@danmar danmar Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

main.cpp is an "example implementation" more or less. It doesn't have to be perfect. If the -std option has been specified then please rely on that primarily.

Cppcheck has both --std and --language. It has better knowledge about the language than simplecpp.
The --language option is not communicated to simplecpp.

@glankk glankk Jul 14, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I suggest something like this:

case simplecpp::Output::PORTABILITY_NO_EOF_NEWLINE:
  if (simplecpp::getCStd(dui.std) == simplecpp::CUnknown) {
      // Only UB for c code, suppress for c++ code
      // If no standard is specified then prefer to have a false negative
      continue;
  }
  std::cerr << "portability: ";
  break;

cppcheck will have it's own way of handling this obviously, but I think this is okay for the simplecpp executable.

@danmar danmar Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know there exists production code where c++ files have a .c extension

yes sure. And in these cases you would use the cppcheck option --language=c++ .

If you just execute cppcheck --std=c++11 path then you say that c++ files in the path should be analyzed using "c++11". and c files in the path are analyzed with default C standard.

break;
case simplecpp::Output::UNHANDLED_CHAR_ERROR:
std::cerr << "unhandled char error: ";
break;
Expand Down
13 changes: 13 additions & 0 deletions simplecpp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,7 @@ static const std::string COMMENT_END("*/");
void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename, OutputList *outputList)
{
unsigned int multiline = 0U;
bool trailing_nl = true;

const Token *oldLastToken = nullptr;

Expand All @@ -671,6 +672,8 @@ void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename,
if (!stream.good())
break;

trailing_nl = false;

if (ch >= 0x80) {
if (outputList) {
simplecpp::Output err{
Expand All @@ -693,6 +696,7 @@ void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename,
} else {
location.line += multiline + 1;
multiline = 0U;
trailing_nl = true;
}
if (!multiline)
location.col = 1;
Expand Down Expand Up @@ -961,6 +965,15 @@ void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename,
location.adjust(currentToken);
}

if (!trailing_nl && outputList) {
Output err{
Output::PORTABILITY_NO_EOF_NEWLINE,
location,
"No newline at end of file."
};
outputList->emplace_back(std::move(err));
}

combineOperators();
}

Expand Down
1 change: 1 addition & 0 deletions simplecpp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ namespace simplecpp {
INCLUDE_NESTED_TOO_DEEPLY,
SYNTAX_ERROR,
PORTABILITY_BACKSLASH,
PORTABILITY_NO_EOF_NEWLINE,
UNHANDLED_CHAR_ERROR,
EXPLICIT_INCLUDE_NOT_FOUND,
FILE_NOT_FOUND,
Expand Down
Loading
Loading