Improve Error Message for Invalid CMake Code

[Skylion007]

// Copyright (c) Facebook, Inc. and its affiliates.
// This source code is licensed under the MIT license found in the
// LICENSE file in the root directory of this source tree.

#define SCENE_DATASETS "${SCENE_DATASETS}"
#define TEST_ASSETS "${TEST_ASSETS}"

#define FILE_THAT_EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/IOTest.cpp"

WARNING __main__.py:530: While processing src/tests/configure.h.cmake
ERROR __main__.py:638: Unexpected UNQUOTED_LITERAL token at 1:0
Traceback (most recent call last):
  File "/private/home/agokaslan/.cache/pre-commit/repoo8nb42vl/py_env-python3/lib/python3.6/site-packages/cmakelang/format/__main__.py", line 633, in main
    return inner_main()
  File "/private/home/agokaslan/.cache/pre-commit/repoo8nb42vl/py_env-python3/lib/python3.6/site-packages/cmakelang/format/__main__.py", line 616, in inner_main
    onefile_main(infile_path, args, argparse_dict)
  File "/private/home/agokaslan/.cache/pre-commit/repoo8nb42vl/py_env-python3/lib/python3.6/site-packages/cmakelang/format/__main__.py", line 526, in onefile_main
    outtext, reflow_valid = process_file(cfg, intext, args.dump)
  File "/private/home/agokaslan/.cache/pre-commit/repoo8nb42vl/py_env-python3/lib/python3.6/site-packages/cmakelang/format/__main__.py", line 158, in process_file
    parse_tree = parse.parse(tokens, ctx)
  File "/private/home/agokaslan/.cache/pre-commit/repoo8nb42vl/py_env-python3/lib/python3.6/site-packages/cmakelang/parse/__init__.py", line 68, in parse
    return BodyNode.consume(ctx, tokens)
  File "/private/home/agokaslan/.cache/pre-commit/repoo8nb42vl/py_env-python3/lib/python3.6/site-packages/cmakelang/parse/body_nodes.py", line 78, in consume
    tokens[0].begin.line, tokens[0].begin.col))
cmakelang.common.InternalError

I am trying to use cmake_format a pre-commit hook per your other repo, but when scanning the file posted above, it crashes. You should adjust the commit hook or the cmake_format accordingly.

[cheshirekow]

This is not a valid cmake file. This looks like a c header which is configured by cmake. Usually I see people name this with a .h.in extension, rather than .h.cmake extension... but in any case, I would not expect cmake-format to do anything but exit with an error code in this case. It looks to me like correct behavior.

Edit: Ok well maybe the error message is a bit obtuse. "invalid cmake code" though is a bit hard to distinguish from other errors, which is why it's classified as an internal error. Perhaps a more helpful error message along the lines of "Is this file actually a cmake file?" could make sense.

[Skylion007]

@cheshirekow You may still want to tighten up the pre-commit hook a bit though.

A Brief Github Code Search reveals about 74K files that use the .h.cmake extension. I doubt very many of them are actually cmake files. 834 use configure.h.cmake explicitly

[cheshirekow]

tighten up the pre-commit hook a bit though.

What specifically are you suggesting?

[Skylion007]

Maybe add a [^.] to the include regex so it won't accept anything would accept configure.cmake as valid, but not configure.h.cmake in the default pre-commit hook include.

[cheshirekow]

Which regex are you referring to?

[Skylion007]

@cheshirekow This one: https://github.com/iconmaster5326/cmake-format-pre-commit-hook/blob/07978b29a9a9fbe98fcd605a74d970a1ce86483f/.pre-commit-hooks.yaml#L6

[cheshirekow]

That's not my repository.

[Skylion007]

Oh whoops, yeah I see you are using the types in yours: https://github.com/cheshirekow/cmake-format-precommit/blob/master/.pre-commit-hooks.yaml#L4 so it's identify problem, not yours. In which, case just improving the error message should be sufficient.

[CAM-Gerlach]

FYI @Skylion007 , Github code search can be a bit misleading, as the filename field unfortunately considers neither wildcards nor (unlike the extension field) exact matches. Of the files whose last extension component is in, 676 000 containh.in, 276 000 contain cmake, while there are 15 500 contain h.cmake.in.

By contrast, of the files with the cmake extension, 50 000 have the h.cmake in the filename, 12 300 have in somewhere in the filename (but the vast majority of those spot-checked aren't in.cmake, but rather have it as part of the filename), and only 2279 have h.in.cmake.

As to the implications for this use case, that's not for me to determine; just wanted to clear up that potential confusion.