improvements (#2)

* add tests (not working yet)

* tests do work

* use yaml config

* fix relpath issue

* fix for .hpp

* improve readme text

* add "catch all" for the default policy

Co-authored-by: Vasily Kulikov <[email protected]>

Author: segoon

README.md

@@ -27,28 +27,76 @@ The tool is simple to use. To sort your #include's just pass it your
 
 ```bash
 cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON ...
-sort-cpp-includes --compile-commands compile_commands.json src/ include/
-sort-cpp-includes --compile-commands compile_commands.json src/ include/
+sort-cpp-includes --compile-commands compile_commands.json src/ include/ main.cpp
 ```
 
-`compile_commands.json` can be generated via `cmake` or vscode.
-After the successful run you might notice changes in your source files.
+`compile_commands.json` can be generated via `cmake` or `vscode`.
+After the successful run you might notice changes in your source files,
+if the sorting order was not met before.
 
+The tool comes with a simple sorting policy: pair header, C headers, C++ headers,
+headers from /usr/include, the rest files. If it doesn't fit you, you may
+define your own policy and pass it to `sort-cpp-includes` using '-d' option.
+An example from Yandex.TPlatform config:
 
-## Implementation
+```yaml
+rules:
+  - matchers:
+      - virtual: "@pair"
+
+  - matchers:
+      - virtual: "@std-c"
+
+  - matchers:
+      - virtual: "@std-cpp"
+
+  - matchers:
+      - regex: "/usr/include/.*"
+
+  - matchers:
+      - regex: ".*/third_party/.*"
+      - regex: ".*/google-benchmark/.*"
+
+  - matchers:
+      - regex: ".*/userver/.*"
+  - matchers:
+      - regex: ".*/build/.*"
+  - matchers:
+      - regex: ".*/libraries/.*"
+  - matchers:
+      - regex: ".*/services/.*"
+```
+
+The sorting and grouping runs in 2 steps. First, the include group is calculated.
+Adjacent include groups will be separated with a single empty line.
+Second, the sorting takes place for each group, independently. Each matcher
+define a sort group. It may consist of one or more matchers.
+For now the following matchers are implemented:
+
+* regex - a simple Python's `re` matcher
+* virtual: @pair - the pair header (`server.hpp` for `server.cpp`)
+* virtual: @std-c - standard C language headers
+* virtual: @std-cpp - standard C++ language headers (C++17)
+
+You may add several matchers to the same group, it would mean "at least one matcher
+from the group must match". The first match wins, IOW, if a first group matcher
+matches the include, the rest groups are skipped.
+
+## Implementation details
 
 To properly split includes into groups separated by newlines, we have to know
 the full paths of the included header files. The full path is used to identify
-an include group according to the user policy. E.g. '#include <os.hpp>` may include
-`$PWD/os.hpp`, `/usr/include/os.hpp`, and so on. sort-cpp-includes doesn't
+an include group according to the user policy. E.g. `#include <os.hpp>` may include
+`$PWD/os.hpp`, `/usr/include/os.hpp`, and so on. `sort-cpp-includes` doesn't
 resolve includes by itself, it uses a C++ compiler's ability to preprocess
 your source files.
 
-
 ## Errata
 
 * Sourceless header files: you have to include each header into matched .cpp files
   at least once.
-* sort-cpp-includes was tested on clang only, so sorting with alternative compilers
+* `sort-cpp-includes` was tested on clang only, so sorting with alternative compilers
   might not work as expected.
-* only a single thread is used, but we could utilize more
+* Only a single thread is used, but we could utilize more
+* `sort-cpp-includes` stops searching include directives on the first non-include line
+  except `#pragma once`.

src/sort_cpp_includes/sort_cpp_includes.py

@@ -10,6 +10,7 @@
 import sys
 import tempfile
 import typing
+import yaml
 
 
 def read_file_contents(path: str) -> str:
@@ -40,6 +41,8 @@ def command_to_cmdline(command):
 
 
 def read_compile_commands(path: str) -> typing.Dict[str, CCEntry]:
+    print('Loading compile_commands.json...')
+
     compile_commands_raw = read_file_contents(path)
     compile_commands_json = json.loads(compile_commands_raw)
 
@@ -51,6 +54,8 @@ def read_compile_commands(path: str) -> typing.Dict[str, CCEntry]:
         )
         for entry in compile_commands_json
     }
+
+    print('compile_commands.json is loaded.')
     return compile_commands
 
 
@@ -346,16 +351,7 @@ def is_match(self, path: str, orig_path: str, my_filename: str) -> bool:
         {'matchers': [{'virtual': '@std-c'}]},
         {'matchers': [{'virtual': '@std-cpp'}]},
         {'matchers': [{'regex': '/usr/include/.*'}]},
-        {
-            'matchers': [
-                {'regex': '.*/third_party/.*'},
-                {'regex': '.*/google-benchmark/.*'},
-            ],
-        },
-        {'matchers': [{'regex': '.*/userver/.*'}]},
-        {'matchers': [{'regex': '.*/build/.*'}]},
-        {'matchers': [{'regex': '.*/libraries/.*'}]},
-        {'matchers': [{'regex': '.*/services/.*'}]},
+        {'matchers': [{'regex': '.*'}]},
     ],
 }
 
@@ -435,11 +431,13 @@ def sort_includes(
                 if match:
                     break
             if match:
+                # print(f'write {line}')
                 res[i].append(line)
                 break
 
         if not match:
             raise Exception(f'Include "{line}" doesn\'t match any pattern')
+    # print(res)
 
     for group in res:
         group.sort(key=lambda x: (not x.endswith('.h>'), x))
@@ -477,6 +475,7 @@ def set(self, key: typing.Any, value: str) -> None:
 
 # Returns the absolute path of a header from 'include_line'
 def include_realpath_cached(
+        filepath: str,
         source_filepath: str,
         include_line: str,
         compile_commands: typing.Dict[str, CCEntry],
@@ -489,30 +488,32 @@ def include_realpath_cached(
     if entry:
         return entry
 
-    result = include_realpath(source_filepath, include_line, compile_commands)
+    result = include_realpath(filepath, source_filepath, include_line, compile_commands)
     if result:
         realpath_cache.set(key, result)
     return result
 
 
 def include_realpath(
+        filepath: str,
         source_filepath: str,
         include_line: str,
         compile_commands: typing.Dict[str, CCEntry],
 ) -> str:
-    filepath = os.path.abspath(source_filepath)
+    filepath = os.path.abspath(filepath)
+    source_filepath = os.path.abspath(source_filepath)
 
-    directory = os.path.dirname(source_filepath)
+    directory = os.path.dirname(filepath)
     tmp = tempfile.NamedTemporaryFile(suffix='.cpp', dir=directory)
     tmp_name = tmp.name
     tmp.write(include_line.encode())
     tmp.write('\n'.encode())
     tmp.flush()
 
-    command = compile_commands.get(filepath)
+    command = compile_commands.get(source_filepath)
     if not command:
         raise Exception(
-            f'Failed to find "{filepath}" in compile_commands.json',
+            f'Failed to find "{source_filepath}" in compile_commands.json',
         )
     command_items = adjust_cc_command(command) + [tmp_name]
 
@@ -650,6 +651,7 @@ def do_handle_single_file(
         if not line.strip():
             continue
         abs_include = include_realpath_cached(
+            filename,
             filename_for_cc, line, compile_commands, realpath_cache,
         )
 
@@ -672,7 +674,7 @@ def do_handle_single_file(
             ofile.write('#pragma once\n\n')
         write_includes(sorted_includes, ofile)
 
-        for line in orig_file_contents[i:]:
+        for line in orig_file_contents[i + 1 :]:
             ofile.write(line)
             ofile.write('\n')
     os.rename(src=tmp_filename, dst=filename)
@@ -684,7 +686,7 @@ def read_config(filepath: typing.Optional[str]) -> Config:
         return Config(DEFAULT_RULES)
 
     with open(filepath, 'r') as ifile:
-        contents = json.load(ifile)
+        contents = yaml.safe_load(ifile)
         print(f'loaded {filepath} rule set')
         return Config(contents)
 
@@ -703,13 +705,15 @@ def collect_files(
 def collect_all_files(
         paths: typing.List[str], suffixes: typing.List[str],
 ) -> typing.List[str]:
+    print('Collecting input files...')
     headers = []
     for filepath in paths:
         if os.path.isfile(filepath):
             headers.append(filepath)
         elif os.path.isdir(filepath):
             for header in collect_files(filepath, suffixes):
                 headers.append(header)
+    print(f'Collected files.')
     return headers
 
 

src/sort_cpp_includes/test_sort.py

@@ -0,0 +1,125 @@
+import typing
+import pytest
+import dataclasses
+import json
+
+from . import sort_cpp_includes
+
+
+@dataclasses.dataclass(frozen=True)
+class FakeArgs:
+    paths: typing.List[str]
+    compile_commands: str
+    config: str
+    hpp_suffixes: str
+    cpp_suffixes: str
+
+
+# TODO: ad-hoc
+COMPILER = '/usr/bin/clang++-9'
+
+
+def compose_compile_commands(args):
+    result = []
+    for arg in args:
+        result.append({
+            'command': ' '.join([f'{COMPILER}', '-c', arg, '-o', f'{arg}.o']),
+            'directory': '/',
+            'file': arg,
+            },
+        )
+    return result
+
+
+@pytest.fixture
+def check(tmp_path):
+    def _check(input_cpp: str, expected_output: str, rules: dict):
+        input_fname = tmp_path / 'input.cpp'
+        with open(input_fname, 'w') as ifile:
+            ifile.write(input_cpp)
+
+        cc = compose_compile_commands([str(input_fname)])
+        cc_fname = tmp_path / 'compile_commands.json'
+        with open(cc_fname, 'w') as cc_file:
+            cc_file.write(json.dumps(cc))
+
+        config_fname = tmp_path / 'sorting-rules.json'
+        with open(config_fname, 'w') as config_file:
+            config_file.write(json.dumps(rules))
+
+        args = FakeArgs(
+                paths=[str(input_fname)],
+                compile_commands=str(cc_fname),
+                config=config_fname,
+                hpp_suffixes='.hpp,.h',
+                cpp_suffixes='.cpp,.cc',
+                )
+        sort_cpp_includes.process(args)
+
+        with open(input_fname, 'r') as ifile:
+            contents = ifile.read()
+
+        # print(contents)
+        # print(expected_output)
+        assert contents.strip() == expected_output.strip()
+
+    return _check
+
+
+def test_one_group(tmp_path, check):
+    input_cpp="""
+#include <iostream>
+#include <atomic>
+#include <cstdio>
+"""
+
+    expected_output="""#include <atomic>
+#include <cstdio>
+#include <iostream>
+
+"""
+
+    rules = {
+            'rules': [
+                {'matchers': [
+                    {'regex': '.*'},
+                    ],
+                    },
+                ],
+            }
+    check(input_cpp, expected_output, rules)
+
+
+def test_multiple_groups(tmp_path, check):
+    input_cpp="""
+#include <iostream>
+#include <time.h>
+#include <atomic>
+#include <locale.h>
+#include <cstdio>
+#include <stdio.h>
+#include <signal.h>
+"""
+
+    expected_output="""
+#include <locale.h>
+#include <signal.h>
+#include <stdio.h>
+#include <time.h>
+
+#include <atomic>
+#include <cstdio>
+#include <iostream>
+"""
+
+    rules = {
+            'rules': [
+        {'matchers': [{'virtual': '@std-c'}]},
+        {'matchers': [{'virtual': '@std-cpp'}]},
+                {'matchers': [
+                    {'regex': '.*'},
+                    ],
+                    },
+                ],
+            }
+    check(input_cpp, expected_output, rules)