-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[clang-tidy] Show descriptive error message when check_alphabetical_order.py fails #170975
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.
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
Conversation
|
@llvm/pr-subscribers-clang-tools-extra Author: Baranov Victor (vbvictor) ChangesFull diff: https://github.com/llvm/llvm-project/pull/170975.diff 2 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/tool/check_alphabetical_order.py b/clang-tools-extra/clang-tidy/tool/check_alphabetical_order.py
index 3220795433187..66819aba435e8 100644
--- a/clang-tools-extra/clang-tidy/tool/check_alphabetical_order.py
+++ b/clang-tools-extra/clang-tidy/tool/check_alphabetical_order.py
@@ -126,9 +126,9 @@ def _scan_bullet_blocks(lines: Sequence[str], start: int, end: int) -> ScannedBl
return ScannedBlocks(blocks_with_pos, i)
-def read_text(path: str) -> List[str]:
+def read_text(path: str) -> str:
with io.open(path, "r", encoding="utf-8") as f:
- return f.read().splitlines(True)
+ return f.read()
def write_text(path: str, content: str) -> None:
@@ -364,14 +364,16 @@ def _emit_duplicate_report(lines: Sequence[str], title: str) -> Optional[str]:
def process_release_notes(out_path: str, rn_doc: str) -> int:
- lines = read_text(rn_doc)
+ text = read_text(rn_doc)
+ lines = text.splitlines(True)
normalized = normalize_release_notes(lines)
write_text(out_path, normalized)
# Prefer reporting ordering issues first; let diff fail the test.
- if "".join(lines) != normalized:
+ if text != normalized:
sys.stderr.write(
- "Note: 'ReleaseNotes.rst' is not normalized; Please fix ordering first.\n"
+ "\nEntries in 'clang-tools-extra/docs/ReleaseNotes.rst' are not alphabetically sorted.\n"
+ "Fix the ordering by applying diff printed below.\n\n"
)
return 0
@@ -383,8 +385,15 @@ def process_release_notes(out_path: str, rn_doc: str) -> int:
def process_checks_list(out_path: str, list_doc: str) -> int:
- lines = read_text(list_doc)
- normalized = normalize_list_rst("".join(lines))
+ text = read_text(list_doc)
+ normalized = normalize_list_rst(text)
+
+ if text != normalized:
+ sys.stderr.write(
+ "\nChecks in 'clang-tools-extra/docs/clang-tidy/checks/list.rst' csv-table are not alphabetically sorted.\n"
+ "Fix the ordering by applying diff printed below.\n\n"
+ )
+
write_text(out_path, normalized)
return 0
diff --git a/clang-tools-extra/clang-tidy/tool/check_alphabetical_order_test.py b/clang-tools-extra/clang-tidy/tool/check_alphabetical_order_test.py
index bbc3dda7d9473..48a3c761c12ce 100644
--- a/clang-tools-extra/clang-tidy/tool/check_alphabetical_order_test.py
+++ b/clang-tools-extra/clang-tidy/tool/check_alphabetical_order_test.py
@@ -164,7 +164,7 @@ def test_process_release_notes_with_unsorted_content(self) -> None:
)
self.assertEqual(out, expected_out)
- self.assertIn("not normalized", buf.getvalue())
+ self.assertIn("not alphabetically sorted", buf.getvalue())
def test_process_release_notes_prioritizes_sorting_over_duplicates(self) -> None:
# Sorting is incorrect and duplicates exist, should report ordering issues first.
@@ -201,7 +201,7 @@ def test_process_release_notes_prioritizes_sorting_over_duplicates(self) -> None
rc = _mod.process_release_notes(out_path, rn_doc)
self.assertEqual(rc, 0)
self.assertIn(
- "Note: 'ReleaseNotes.rst' is not normalized; Please fix ordering first.",
+ "Entries in 'clang-tools-extra/docs/ReleaseNotes.rst' are not alphabetically sorted.",
buf.getvalue(),
)
@@ -371,7 +371,14 @@ def test_process_checks_list_normalizes_output(self) -> None:
out_doc = os.path.join(td, "out.rst")
with open(in_doc, "w", encoding="utf-8") as f:
f.write(list_text)
- rc = _mod.process_checks_list(out_doc, in_doc)
+ buf = io.StringIO()
+ with redirect_stderr(buf):
+ rc = _mod.process_checks_list(out_doc, in_doc)
+ self.assertEqual(rc, 0)
+ self.assertIn(
+ "Checks in 'clang-tools-extra/docs/clang-tidy/checks/list.rst' csv-table are not alphabetically sorted.",
+ buf.getvalue(),
+ )
self.assertEqual(rc, 0)
with open(out_doc, "r", encoding="utf-8") as f:
out = f.read()
|
|
@llvm/pr-subscribers-clang-tidy Author: Baranov Victor (vbvictor) ChangesFull diff: https://github.com/llvm/llvm-project/pull/170975.diff 2 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/tool/check_alphabetical_order.py b/clang-tools-extra/clang-tidy/tool/check_alphabetical_order.py
index 3220795433187..66819aba435e8 100644
--- a/clang-tools-extra/clang-tidy/tool/check_alphabetical_order.py
+++ b/clang-tools-extra/clang-tidy/tool/check_alphabetical_order.py
@@ -126,9 +126,9 @@ def _scan_bullet_blocks(lines: Sequence[str], start: int, end: int) -> ScannedBl
return ScannedBlocks(blocks_with_pos, i)
-def read_text(path: str) -> List[str]:
+def read_text(path: str) -> str:
with io.open(path, "r", encoding="utf-8") as f:
- return f.read().splitlines(True)
+ return f.read()
def write_text(path: str, content: str) -> None:
@@ -364,14 +364,16 @@ def _emit_duplicate_report(lines: Sequence[str], title: str) -> Optional[str]:
def process_release_notes(out_path: str, rn_doc: str) -> int:
- lines = read_text(rn_doc)
+ text = read_text(rn_doc)
+ lines = text.splitlines(True)
normalized = normalize_release_notes(lines)
write_text(out_path, normalized)
# Prefer reporting ordering issues first; let diff fail the test.
- if "".join(lines) != normalized:
+ if text != normalized:
sys.stderr.write(
- "Note: 'ReleaseNotes.rst' is not normalized; Please fix ordering first.\n"
+ "\nEntries in 'clang-tools-extra/docs/ReleaseNotes.rst' are not alphabetically sorted.\n"
+ "Fix the ordering by applying diff printed below.\n\n"
)
return 0
@@ -383,8 +385,15 @@ def process_release_notes(out_path: str, rn_doc: str) -> int:
def process_checks_list(out_path: str, list_doc: str) -> int:
- lines = read_text(list_doc)
- normalized = normalize_list_rst("".join(lines))
+ text = read_text(list_doc)
+ normalized = normalize_list_rst(text)
+
+ if text != normalized:
+ sys.stderr.write(
+ "\nChecks in 'clang-tools-extra/docs/clang-tidy/checks/list.rst' csv-table are not alphabetically sorted.\n"
+ "Fix the ordering by applying diff printed below.\n\n"
+ )
+
write_text(out_path, normalized)
return 0
diff --git a/clang-tools-extra/clang-tidy/tool/check_alphabetical_order_test.py b/clang-tools-extra/clang-tidy/tool/check_alphabetical_order_test.py
index bbc3dda7d9473..48a3c761c12ce 100644
--- a/clang-tools-extra/clang-tidy/tool/check_alphabetical_order_test.py
+++ b/clang-tools-extra/clang-tidy/tool/check_alphabetical_order_test.py
@@ -164,7 +164,7 @@ def test_process_release_notes_with_unsorted_content(self) -> None:
)
self.assertEqual(out, expected_out)
- self.assertIn("not normalized", buf.getvalue())
+ self.assertIn("not alphabetically sorted", buf.getvalue())
def test_process_release_notes_prioritizes_sorting_over_duplicates(self) -> None:
# Sorting is incorrect and duplicates exist, should report ordering issues first.
@@ -201,7 +201,7 @@ def test_process_release_notes_prioritizes_sorting_over_duplicates(self) -> None
rc = _mod.process_release_notes(out_path, rn_doc)
self.assertEqual(rc, 0)
self.assertIn(
- "Note: 'ReleaseNotes.rst' is not normalized; Please fix ordering first.",
+ "Entries in 'clang-tools-extra/docs/ReleaseNotes.rst' are not alphabetically sorted.",
buf.getvalue(),
)
@@ -371,7 +371,14 @@ def test_process_checks_list_normalizes_output(self) -> None:
out_doc = os.path.join(td, "out.rst")
with open(in_doc, "w", encoding="utf-8") as f:
f.write(list_text)
- rc = _mod.process_checks_list(out_doc, in_doc)
+ buf = io.StringIO()
+ with redirect_stderr(buf):
+ rc = _mod.process_checks_list(out_doc, in_doc)
+ self.assertEqual(rc, 0)
+ self.assertIn(
+ "Checks in 'clang-tools-extra/docs/clang-tidy/checks/list.rst' csv-table are not alphabetically sorted.",
+ buf.getvalue(),
+ )
self.assertEqual(rc, 0)
with open(out_doc, "r", encoding="utf-8") as f:
out = f.read()
|
| def read_text(path: str) -> str: | ||
| with io.open(path, "r", encoding="utf-8") as f: | ||
| return f.read().splitlines(True) | ||
| return f.read() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought to make it consistent with write_text because it accepts content: str
zeyi2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you :)
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/17757 Here is the relevant piece of the build log for the reference |
Now we give clearer instructions on how to interpret the output of the script to fix issues it reported