From 9e76f819d9fe6bcee67d61beb716ad459ed7cc97 Mon Sep 17 00:00:00 2001 From: shrivas Date: Wed, 17 Sep 2025 10:34:25 +0530 Subject: [PATCH 1/8] check_spdx script fix --- scripts/check_spdx.py | 73 +++++++---------------------- src/moldflow/boundary_conditions.py | 3 -- 2 files changed, 16 insertions(+), 60 deletions(-) diff --git a/scripts/check_spdx.py b/scripts/check_spdx.py index 25f8577..838fb66 100644 --- a/scripts/check_spdx.py +++ b/scripts/check_spdx.py @@ -68,33 +68,6 @@ def has_spdx_header(content: str) -> bool: first_lines = content.split('\n')[:3] return any('SPDX-License-Identifier' in line for line in first_lines) -def add_spdx_header(file_path: str) -> bool: - """Add SPDX header to a file if it doesn't have one.""" - try: - with open(file_path, 'r', encoding='utf-8') as f: - content = f.read() - - if has_spdx_header(content): - print(f"Skipping {file_path} - already has SPDX header") - return False - - # If there's a shebang, preserve it - if content.startswith('#!'): - shebang, rest = content.split('\n', 1) - new_content = f"{shebang}\n{SPDX_HEADER}{rest}" - else: - new_content = SPDX_HEADER + content - - with open(file_path, 'w', encoding='utf-8') as f: - f.write(new_content) - - print(f"Added SPDX header to {file_path}") - return True - - except Exception as e: - print(f"Error processing {file_path}: {e}", file=sys.stderr) - return False - def find_python_files(start_dir: str, gitignore_spec: pathspec.PathSpec) -> List[str]: """Find all Python files in the directory tree.""" python_files = [] @@ -110,10 +83,6 @@ def find_python_files(start_dir: str, gitignore_spec: pathspec.PathSpec) -> List def main(): """Main entry point.""" - parser = argparse.ArgumentParser(description="SPDX header checker/adder") - parser.add_argument("--check-only", action="store_true", help="Only check, do not modify files") - args = parser.parse_args() - # Get repository root (assumes script is in scripts/ directory) repo_root = str(Path(__file__).parent.parent) @@ -124,33 +93,23 @@ def main(): python_files = find_python_files(repo_root, gitignore_spec) missing = [] - modified = 0 - - if args.check_only: - for file_path in python_files: - try: - with open(file_path, 'r', encoding='utf-8') as f: - content = f.read() - if not has_spdx_header(content): - missing.append(file_path) - except Exception as e: # pragma: no cover - print(f"Error reading {file_path}: {e}", file=sys.stderr) - if missing: - print("Files missing SPDX headers:") - for p in missing: - print(f" {p}") - print(f"\nChecked {len(python_files)} files; {len(missing)} missing headers") - return 1 - print(f"Checked {len(python_files)} files; all have SPDX headers") - return 0 - else: - for file_path in python_files: - if add_spdx_header(file_path): - modified += 1 - print(f"\nProcessed {len(python_files)} files") - print(f"Added SPDX headers to {modified} files") - return 0 + for file_path in python_files: + try: + with open(file_path, 'r', encoding='utf-8') as f: + content = f.read() + if not has_spdx_header(content): + missing.append(file_path) + except Exception as e: # pragma: no cover + print(f"Error reading {file_path}: {e}", file=sys.stderr) + if missing: + print("Files missing SPDX headers:") + for p in missing: + print(f" {p}") + print(f"\nChecked {len(python_files)} files; {len(missing)} missing headers") + return 1 + print(f"Checked {len(python_files)} files; all have SPDX headers") + return 0 if __name__ == '__main__': main() diff --git a/src/moldflow/boundary_conditions.py b/src/moldflow/boundary_conditions.py index e01bb13..a8b0e56 100644 --- a/src/moldflow/boundary_conditions.py +++ b/src/moldflow/boundary_conditions.py @@ -1,6 +1,3 @@ -# SPDX-FileCopyrightText: 2025 Autodesk, Inc. -# SPDX-License-Identifier: Apache-2.0 - """ Usage: BoundaryConditions Class API Wrapper From 3098ad536b7510cda39c51d34c14b9d5b60bfd2b Mon Sep 17 00:00:00 2001 From: shrivas Date: Wed, 17 Sep 2025 10:53:11 +0530 Subject: [PATCH 2/8] Update ci --- .github/workflows/ci.yml | 4 ++-- scripts/check_spdx.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1f7132d..16e3571 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,8 +27,8 @@ jobs: python -m pip install --upgrade pip python -m pip install -r requirements.txt - - name: SPDX headers check (check-only) - run: python scripts/check_spdx.py --check-only + - name: SPDX headers check + run: python scripts/check_spdx.py - name: Localization check run: python scripts/check_localization.py --check-only --path src/moldflow --locale-path src/moldflow/locale diff --git a/scripts/check_spdx.py b/scripts/check_spdx.py index 838fb66..8fecdf8 100644 --- a/scripts/check_spdx.py +++ b/scripts/check_spdx.py @@ -112,4 +112,4 @@ def main(): return 0 if __name__ == '__main__': - main() + sys.exit(main()) From f2b35eaa61b1795b5039a94ab2651838d4cf608b Mon Sep 17 00:00:00 2001 From: shrivas Date: Wed, 17 Sep 2025 10:57:23 +0530 Subject: [PATCH 3/8] Checking positive --- src/moldflow/boundary_conditions.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/moldflow/boundary_conditions.py b/src/moldflow/boundary_conditions.py index a8b0e56..e01bb13 100644 --- a/src/moldflow/boundary_conditions.py +++ b/src/moldflow/boundary_conditions.py @@ -1,3 +1,6 @@ +# SPDX-FileCopyrightText: 2025 Autodesk, Inc. +# SPDX-License-Identifier: Apache-2.0 + """ Usage: BoundaryConditions Class API Wrapper From 126de6cbc5c5acfcc472b1ce1caf7e151801c349 Mon Sep 17 00:00:00 2001 From: shrivas Date: Wed, 17 Sep 2025 11:01:33 +0530 Subject: [PATCH 4/8] Checking new CI --- .github/workflows/ci.yml | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 16e3571..5c78633 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,6 +7,29 @@ on: types: [ published ] jobs: + # Static checks that don't require building + static-checks: + runs-on: windows-2022 + steps: + - uses: actions/checkout@v5 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.13' + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + python -m pip install -r requirements.txt + + - name: SPDX headers check + run: python scripts/check_spdx.py + + - name: Localization check + run: python scripts/check_localization.py --check-only --path src/moldflow --locale-path src/moldflow/locale + + # Build and test matrix test: runs-on: ${{ matrix.os }} strategy: @@ -27,12 +50,6 @@ jobs: python -m pip install --upgrade pip python -m pip install -r requirements.txt - - name: SPDX headers check - run: python scripts/check_spdx.py - - - name: Localization check - run: python scripts/check_localization.py --check-only --path src/moldflow --locale-path src/moldflow/locale - - name: Build package run: python run.py build --install @@ -47,7 +64,6 @@ jobs: build-docs: runs-on: windows-2022 - needs: test steps: - uses: actions/checkout@v5 From 73d638d0babdc997fea6711955671d59556ea41c Mon Sep 17 00:00:00 2001 From: shrivas Date: Wed, 17 Sep 2025 11:44:29 +0530 Subject: [PATCH 5/8] import fix --- scripts/check_spdx.py | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/check_spdx.py b/scripts/check_spdx.py index 8fecdf8..c027fa6 100644 --- a/scripts/check_spdx.py +++ b/scripts/check_spdx.py @@ -8,7 +8,6 @@ import os import sys -import argparse from pathlib import Path from typing import List import pathspec From 64b3bb1d054332aeca7ad37bc56faab78cbd17d5 Mon Sep 17 00:00:00 2001 From: shrivas Date: Wed, 17 Sep 2025 11:45:12 +0530 Subject: [PATCH 6/8] lint fix for scripts --- run.py | 3 +- scripts/check_localization.py | 546 ++++++++++++++++++++++------------ scripts/check_spdx.py | 20 +- 3 files changed, 376 insertions(+), 193 deletions(-) diff --git a/run.py b/run.py index 61a32db..40f41a1 100644 --- a/run.py +++ b/run.py @@ -86,6 +86,7 @@ DOCS_BUILD_DIR = os.path.join(DOCS_DIR, 'build') COVERAGE_HTML_DIR = os.path.join(ROOT_DIR, 'htmlcov') DIST_DIR = os.path.join(ROOT_DIR, 'dist') +SCRIPTS_DIR = os.path.join(ROOT_DIR, 'scripts') # Files PYLINT_CONFIG_FILE = os.path.join(ROOT_DIR, '.pylint.toml') @@ -96,7 +97,7 @@ COVERAGE_XML_FILE_NAME = 'coverage.xml' VERSION_FILE = os.path.join(ROOT_DIR, VERSION_JSON) DIST_FILES = os.path.join(ROOT_DIR, 'dist', '*') -PYTHON_FILES = [MOLDFLOW_DIR, DOCS_SOURCE_DIR, TEST_DIR, "run.py"] +PYTHON_FILES = [MOLDFLOW_DIR, DOCS_SOURCE_DIR, TEST_DIR, "run.py", SCRIPTS_DIR] def run_command(args, cwd=os.getcwd(), extra_env=None): diff --git a/scripts/check_localization.py b/scripts/check_localization.py index c0be77b..0bd771a 100644 --- a/scripts/check_localization.py +++ b/scripts/check_localization.py @@ -40,6 +40,8 @@ @dataclass class PoEntry: + """Entry in a .po file.""" + msgid: str msgstr: str line_no: int @@ -47,6 +49,8 @@ class PoEntry: @dataclass class LocalizationFix: + """Fix for a localization issue.""" + string_value: str source_file: str line_no: int @@ -156,19 +160,25 @@ def write_back(self) -> None: if line.strip().startswith('msgid ""') and i > 0: # Find the end of the header entry j = i + 1 - while j < len(lines) and (lines[j].strip().startswith('msgstr') or lines[j].strip().startswith('"')): + while j < len(lines) and ( + lines[j].strip().startswith('msgstr') or lines[j].strip().startswith('"') + ): j += 1 header_end = j break # Preserve header - header_lines = lines[:header_end] if header_end > 0 else [ - 'msgid ""', - 'msgstr ""', - '"Content-Type: text/plain; charset=UTF-8\\n"', - f'"Language: {self.po_file_path.parent.parent.name}\\n"', - '' - ] + header_lines = ( + lines[:header_end] + if header_end > 0 + else [ + 'msgid ""', + 'msgstr ""', + '"Content-Type: text/plain; charset=UTF-8\\n"', + f'"Language: {self.po_file_path.parent.parent.name}\\n"', + '', + ] + ) # Sort entries for consistent output sorted_entries = sorted(self.entries.values(), key=lambda x: x.msgid) @@ -216,79 +226,130 @@ def _load_po_files(self): # Create parser for new locale self.po_files[locale_dir.name] = PoFileParser(po_file) + def _is_underscore_call(self, call_node: ast.Call) -> bool: + """Check if this is a _() function call.""" + return isinstance(call_node.func, ast.Name) and call_node.func.id == "_" + + def _is_get_text_call(self, call_node: ast.Call) -> bool: + """Check if this is a get_text()() function call.""" + return ( + isinstance(call_node.func, ast.Call) + and isinstance(call_node.func.func, ast.Name) + and call_node.func.func.id == "get_text" + ) + + def _extract_string_from_call(self, call_node: ast.Call) -> str | None: + """Extract string value from function call if it's a string constant.""" + if ( + call_node.args + and isinstance(call_node.args[0], ast.Constant) + and isinstance(call_node.args[0].value, str) + ): + return call_node.args[0].value + return None + def _extract_strings_from_calls(self, node: ast.AST) -> List[Tuple[str, int, str]]: """Extract string literals from _() function calls and process_log usage.""" strings = [] for child in ast.walk(node): - if isinstance(child, ast.Call): - # Check for _() calls - if isinstance(child.func, ast.Name) and child.func.id == "_": - if child.args and isinstance(child.args[0], ast.Constant) and isinstance(child.args[0].value, str): - strings.append((child.args[0].value, child.lineno, "_() call")) - - # Check for get_text()() calls - elif (isinstance(child.func, ast.Call) and - isinstance(child.func.func, ast.Name) and - child.func.func.id == "get_text"): - if child.args and isinstance(child.args[0], ast.Constant) and isinstance(child.args[0].value, str): - strings.append((child.args[0].value, child.lineno, "get_text()() call")) - - # Check for process_log calls to see which enum messages are actually used - elif isinstance(child.func, ast.Name) and child.func.id == "process_log": - if len(child.args) >= 2: - # Second argument should be LogMessage enum - message_arg = child.args[1] - if isinstance(message_arg, ast.Attribute): - # Look for LogMessage.SOME_MESSAGE pattern - if (isinstance(message_arg.value, ast.Name) and - message_arg.value.id == "LogMessage"): - # This indicates LogMessage.SOME_MESSAGE is being used - # We'll catch the actual string value from the enum definition - pass + if not isinstance(child, ast.Call): + continue + + # Check for _() calls + if self._is_underscore_call(child): + if string_value := self._extract_string_from_call(child): + strings.append((string_value, child.lineno, "_() call")) + + # Check for get_text()() calls + elif self._is_get_text_call(child): + if string_value := self._extract_string_from_call(child): + strings.append((string_value, child.lineno, "get_text()() call")) + + # Check for process_log calls to see which enum messages are actually used + elif isinstance(child.func, ast.Name) and child.func.id == "process_log": + if len(child.args) >= 2: + # Second argument should be LogMessage enum + message_arg = child.args[1] + if isinstance(message_arg, ast.Attribute): + # Look for LogMessage.SOME_MESSAGE pattern + if ( + isinstance(message_arg.value, ast.Name) + and message_arg.value.id == "LogMessage" + ): + # This indicates LogMessage.SOME_MESSAGE is being used + # We'll catch the actual string value from the enum definition + pass return strings def _extract_enum_strings(self, node: ast.AST) -> List[Tuple[str, int, str]]: """Extract string values from specific message enum definitions.""" strings = [] - - # Only target specific enum classes that contain user-facing messages target_enums = {"ErrorMessage", "LogMessage", "ValueErrorReason"} - # Look for class definitions that might be enums for child in ast.walk(node): - if isinstance(child, ast.ClassDef) and child.name in target_enums: - # Check if it inherits from Enum - is_enum = any( - (isinstance(base, ast.Name) and base.id == "Enum") or - (isinstance(base, ast.Attribute) and base.attr == "Enum") - for base in child.bases - ) + if not isinstance(child, ast.ClassDef) or child.name not in target_enums: + continue + + if not self._is_enum_class(child): + continue + + strings.extend(self._extract_strings_from_enum_body(child)) + + return strings + + def _is_enum_class(self, class_node: ast.ClassDef) -> bool: + """Check if a class inherits from Enum.""" + return any( + (isinstance(base, ast.Name) and base.id == "Enum") + or (isinstance(base, ast.Attribute) and base.attr == "Enum") + for base in class_node.bases + ) + + def _extract_strings_from_enum_body( + self, enum_class: ast.ClassDef + ) -> List[Tuple[str, int, str]]: + """Extract strings from enum class body.""" + strings = [] + + for item in enum_class.body: + if not isinstance(item, ast.Assign): + continue + + for target in item.targets: + if not isinstance(target, ast.Name): + continue - if is_enum: - # Extract string values from enum members - for item in child.body: - if isinstance(item, ast.Assign): - for target in item.targets: - if isinstance(target, ast.Name): - if isinstance(item.value, ast.Constant) and isinstance(item.value.value, str): - strings.append(( - item.value.value, - item.lineno, - f"Enum {child.name}.{target.id}" - )) - elif isinstance(item.value, ast.Tuple): - # Handle tuples like (message, log_level) - extract first element (message) - if item.value.elts and isinstance(item.value.elts[0], ast.Constant) and isinstance(item.value.elts[0].value, str): - strings.append(( - item.value.elts[0].value, - item.lineno, - f"Enum {child.name}.{target.id} message" - )) + string_info = self._extract_string_from_enum_value(item, enum_class.name, target.id) + if string_info: + strings.append(string_info) return strings + def _extract_string_from_enum_value( + self, assign_node: ast.Assign, class_name: str, target_name: str + ) -> Tuple[str, int, str] | None: + """Extract string from enum assignment value.""" + # Handle direct string assignment + if isinstance(assign_node.value, ast.Constant) and isinstance(assign_node.value.value, str): + return (assign_node.value.value, assign_node.lineno, f"Enum {class_name}.{target_name}") + + # Handle tuple assignment (message, log_level) + if isinstance(assign_node.value, ast.Tuple): + if ( + assign_node.value.elts + and isinstance(assign_node.value.elts[0], ast.Constant) + and isinstance(assign_node.value.elts[0].value, str) + ): + return ( + assign_node.value.elts[0].value, + assign_node.lineno, + f"Enum {class_name}.{target_name} message", + ) + + return None + def check_file(self, file_path: Path) -> Tuple[List[str], List[LocalizationFix]]: """Check a single Python file for localization issues.""" try: @@ -316,20 +377,22 @@ def check_file(self, file_path: Path) -> Tuple[List[str], List[LocalizationFix]] for string_value, line_no, context in all_strings: if not base_po.has_string(string_value): - violations.append(f"{file_path}:{line_no}: missing localization for '{string_value}' ({context})") - fixes.append(LocalizationFix( - string_value=string_value, - source_file=str(file_path), - line_no=line_no, - context=context - )) + violations.append( + f"{file_path}:{line_no}: missing localization for '{string_value}' ({context})" + ) + fixes.append( + LocalizationFix( + string_value=string_value, + source_file=str(file_path), + line_no=line_no, + context=context, + ) + ) return violations, fixes def check_translation_gaps(self) -> List[str]: """Check which English strings are missing translations in other locale files.""" - violations = [] - base_po = self.po_files.get(self.base_locale) if not base_po: return [f"Base locale {self.base_locale} not found"] @@ -340,60 +403,117 @@ def check_translation_gaps(self) -> List[str]: if total_locales == 0: return [] - # Collect statistics + locale_stats = self._collect_translation_stats(base_po, total_strings) + violations = [] + + if self._has_translation_issues(locale_stats): + violations.extend(self._generate_summary_report(locale_stats, total_strings)) + violations.extend(self._generate_detailed_report(locale_stats)) + + return violations + + def _collect_translation_stats(self, base_po, total_strings: int) -> dict: + """Collect translation statistics for all locales.""" locale_stats = {} - # Check each non-English locale for locale_name, po_parser in self.po_files.items(): if locale_name == self.base_locale: continue - missing_translations = [] - empty_translations = [] - - # Check each string in the base locale - for msgid, entry in base_po.entries.items(): - if msgid: # Skip empty msgid (header) - if not po_parser.has_string(msgid): - missing_translations.append(msgid) - else: - # Check if translation is empty or same as source (untranslated) - target_entry = po_parser.entries[msgid] - if not target_entry.msgstr.strip() or target_entry.msgstr == msgid: - empty_translations.append(msgid) + missing_translations, empty_translations = self._analyze_locale_translations( + base_po, po_parser + ) locale_stats[locale_name] = { 'missing': missing_translations, 'empty': empty_translations, - 'translated': total_strings - len(missing_translations) - len(empty_translations) + 'translated': total_strings - len(missing_translations) - len(empty_translations), } - # Add summary - if any(locale_stats[locale]['missing'] or locale_stats[locale]['empty'] for locale in locale_stats): - violations.append(f"Translation Status Summary (Base: {self.base_locale}, {total_strings} total strings):") - for locale_name in sorted(locale_stats.keys()): - stats = locale_stats[locale_name] - completion = (stats['translated'] / total_strings) * 100 if total_strings > 0 else 0 - violations.append(f" {locale_name}: {completion:.1f}% complete ({stats['translated']}/{total_strings} translated, {len(stats['missing'])} missing, {len(stats['empty'])} untranslated)") - violations.append("") # Empty line for readability + return locale_stats + + def _analyze_locale_translations(self, base_po, po_parser) -> tuple[list[str], list[str]]: + """Analyze translations for a single locale.""" + missing_translations = [] + empty_translations = [] + + for msgid, _ in base_po.entries.items(): + if not msgid: # Skip empty msgid (header) + continue + + if not po_parser.has_string(msgid): + missing_translations.append(msgid) + else: + target_entry = po_parser.entries[msgid] + if not target_entry.msgstr.strip() or target_entry.msgstr == msgid: + empty_translations.append(msgid) + + return missing_translations, empty_translations + + def _has_translation_issues(self, locale_stats: dict) -> bool: + """Check if any locale has translation issues.""" + return any(stats['missing'] or stats['empty'] for stats in locale_stats.values()) + + def _generate_summary_report(self, locale_stats: dict, total_strings: int) -> list[str]: + """Generate summary report of translation status.""" + violations = [ + "Translation Status Summary " + f"(Base: {self.base_locale}, {total_strings} total strings):" + ] + + for locale_name in sorted(locale_stats.keys()): + stats = locale_stats[locale_name] + completion = (stats['translated'] / total_strings) * 100 if total_strings > 0 else 0 + violations.append( + f" {locale_name}: {completion:.1f}% complete " + f"({stats['translated']}/{total_strings} translated, " + f"{len(stats['missing'])} missing, " + f"{len(stats['empty'])} untranslated)" + ) + + violations.append("") # Empty line for readability + return violations + + def _generate_detailed_report(self, locale_stats: dict) -> list[str]: + """Generate detailed breakdown for each locale.""" + violations = [] - # Detailed breakdown for each locale for locale_name in sorted(locale_stats.keys()): stats = locale_stats[locale_name] + violations.extend(self._report_missing_strings(locale_name, stats['missing'])) + violations.extend(self._report_empty_strings(locale_name, stats['empty'])) - if stats['missing']: - violations.append(f"Locale {locale_name}: {len(stats['missing'])} strings not present in .po file") - for msgid in stats['missing'][:5]: # Show first 5 as examples - violations.append(f" Missing: '{msgid}'") - if len(stats['missing']) > 5: - violations.append(f" ... and {len(stats['missing']) - 5} more") + return violations - if stats['empty']: - violations.append(f"Locale {locale_name}: {len(stats['empty'])} strings need translation") - for msgid in stats['empty'][:3]: # Show first 3 as examples - violations.append(f" Untranslated: '{msgid}'") - if len(stats['empty']) > 3: - violations.append(f" ... and {len(stats['empty']) - 3} more") + def _report_missing_strings(self, locale_name: str, missing_strings: list[str]) -> list[str]: + """Report missing strings for a locale.""" + if not missing_strings: + return [] + + violations = [ + f"Locale {locale_name}: {len(missing_strings)} strings not present in .po file" + ] + + for msgid in missing_strings[:5]: # Show first 5 as examples + violations.append(f" Missing: '{msgid}'") + + if len(missing_strings) > 5: + violations.append(f" ... and {len(missing_strings) - 5} more") + + return violations + + def _report_empty_strings(self, locale_name: str, empty_strings: list[str]) -> list[str]: + """Report empty/untranslated strings for a locale.""" + if not empty_strings: + return [] + + violations = [f"Locale {locale_name}: {len(empty_strings)} strings need translation"] + + for msgid in empty_strings[:3]: # Show first 3 as examples + violations.append(f" Untranslated: '{msgid}'") + + if len(empty_strings) > 3: + violations.append(f" ... and {len(empty_strings) - 3} more") return violations @@ -415,16 +535,21 @@ def apply_fixes(self, fixes: List[LocalizationFix]) -> int: if base_po: for string_value, fix in unique_strings.items(): if not base_po.has_string(string_value): - base_po.add_entry(string_value, string_value) # English uses same for msgid and msgstr + base_po.add_entry( + string_value, string_value + ) # English uses same for msgid and msgstr applied_count += 1 - print(f"Added to {self.base_locale}: '{string_value}' (from {fix.source_file}:{fix.line_no})") + print( + f"Added to {self.base_locale}: " + f"'{string_value}' (from {fix.source_file}:{fix.line_no})" + ) base_po.write_back() # Add to other locales (with English text as fallback) for locale_name, po_parser in self.po_files.items(): if locale_name != self.base_locale: - for string_value in unique_strings.keys(): + for string_value in unique_strings: if not po_parser.has_string(string_value): po_parser.add_entry(string_value, string_value) # Use English as fallback print(f"Added to {locale_name}: '{string_value}' (fallback: English)") @@ -496,8 +621,10 @@ def check_orphaned_strings(self) -> List[str]: orphaned_strings.append(msgid) if orphaned_strings: - violations.append(f"\nLocale {locale_name}: {len(orphaned_strings)} orphaned strings") - for i, msgid in enumerate(orphaned_strings[:3]): + violations.append( + f"\nLocale {locale_name}: {len(orphaned_strings)} orphaned strings" + ) + for msgid in orphaned_strings[:3]: violations.append(f" Orphaned: '{msgid}'") if len(orphaned_strings) > 3: violations.append(f" ... and {len(orphaned_strings) - 3} more") @@ -538,19 +665,20 @@ def apply_orphaned_string_cleanup(self) -> int: return removed_count -def main() -> int: +def _create_argument_parser() -> argparse.ArgumentParser: + """Create and configure the argument parser.""" parser = argparse.ArgumentParser(description=__doc__) parser.add_argument( "--path", type=Path, default=Path("src/moldflow"), - help="Root directory to scan for Python files containing message enums and localization calls", + help="Root directory to scan for Python files with message enums and localization calls", ) parser.add_argument( - "--locale-path", + "--locale-path", type=Path, default=Path("src/moldflow/locale"), - help="Root directory containing locale files" + help="Root directory containing locale files", ) parser.add_argument( "--check-only", @@ -562,33 +690,21 @@ def main() -> int: action="store_true", help="Skip adding missing message strings from source code", ) + parser.add_argument("--no-fix-gaps", action="store_true", help="Skip fixing translation gaps") parser.add_argument( - "--no-fix-gaps", - action="store_true", - help="Skip fixing translation gaps", - ) - parser.add_argument( - "--no-cleanup-orphaned", - action="store_true", - help="Skip removing orphaned strings", + "--no-cleanup-orphaned", action="store_true", help="Skip removing orphaned strings" ) parser.add_argument( "--translation-gaps-only", action="store_true", help="Only check for translation gaps in locale files, skip source code analysis", ) - args = parser.parse_args() + return parser - src_root: Path = args.path - locale_root: Path = args.locale_path - # Determine which operations to run (all enabled by default unless explicitly disabled) - run_autofix = not args.check_only and not args.no_autofix and not args.translation_gaps_only - run_fix_gaps = not args.check_only and not args.no_fix_gaps - run_cleanup_orphaned = not args.check_only and not args.no_cleanup_orphaned - - # Only require source path if not just checking translation gaps - if not args.translation_gaps_only and not src_root.exists(): +def _validate_paths(src_root: Path, locale_root: Path, translation_gaps_only: bool) -> int: + """Validate that required paths exist.""" + if not translation_gaps_only and not src_root.exists(): print(f"Source path not found: {src_root}", file=sys.stderr) return 2 @@ -596,54 +712,66 @@ def main() -> int: print(f"Locale path not found: {locale_root}", file=sys.stderr) return 2 - checker = LocalizationChecker(src_root, locale_root) + return 0 - # Show what operations will be performed - if not args.check_only: - operations = [] - if run_autofix: - operations.append("autofix (add missing strings)") - if run_fix_gaps: - operations.append("fix-gaps (fill translations)") - if run_cleanup_orphaned: - operations.append("cleanup-orphaned (remove stale strings)") - if operations: - print(f"Running localization management: {', '.join(operations)}") - else: - print("Running localization check only (no fixes will be applied)") - print() +def _show_operations( + check_only: bool, run_autofix: bool, run_fix_gaps: bool, run_cleanup_orphaned: bool +) -> None: + """Show what operations will be performed.""" + if check_only: + return + + operations = [] + if run_autofix: + operations.append("autofix (add missing strings)") + if run_fix_gaps: + operations.append("fix-gaps (fill translations)") + if run_cleanup_orphaned: + operations.append("cleanup-orphaned (remove stale strings)") + + if operations: + print(f"Running localization management: {', '.join(operations)}") + else: + print("Running localization check only (no fixes will be applied)") + print() + +def _get_python_files(src_root: Path) -> list[Path]: + """Get list of Python files to process.""" + if src_root.is_file(): + if src_root.suffix == ".py": + return [src_root] + print(f"File is not a Python file: {src_root}", file=sys.stderr) + raise SystemExit(2) + return [p for p in src_root.rglob("*.py") if p.is_file()] + + +def _process_source_files( + checker: LocalizationChecker, py_files: list[Path], run_autofix: bool +) -> list[str]: + """Process source files and apply fixes if needed.""" all_violations = [] all_fixes = [] - # Skip source code analysis if only checking translation gaps - if not args.translation_gaps_only: - # Handle single file vs directory - if src_root.is_file(): - if src_root.suffix == ".py": - py_files = [src_root] - else: - print(f"File is not a Python file: {src_root}", file=sys.stderr) - return 2 - else: - py_files = [p for p in src_root.rglob("*.py") if p.is_file()] + for py_file in py_files: + violations, fixes = checker.check_file(py_file) + all_violations.extend(violations) + all_fixes.extend(fixes) + + # Apply fixes if autofix is enabled + if run_autofix and all_fixes: + applied_count = checker.apply_fixes(all_fixes) + if applied_count > 0: + print(f"\nAdded {applied_count} missing strings to locale files.") - for py_file in py_files: - violations, fixes = checker.check_file(py_file) - all_violations.extend(violations) - all_fixes.extend(fixes) + return all_violations - # Apply fixes if autofix is enabled - if run_autofix and all_fixes: - applied_count = checker.apply_fixes(all_fixes) - if applied_count > 0: - print(f"\nAdded {applied_count} missing strings to locale files.") - # Check for translation gaps in other locales +def _handle_translation_gaps(checker: LocalizationChecker, run_fix_gaps: bool) -> list[str]: + """Handle translation gaps checking and fixing.""" translation_gaps = checker.check_translation_gaps() - # Apply translation gap fixes if enabled if run_fix_gaps and translation_gaps: gap_fixes_applied = checker.apply_translation_gap_fixes() if gap_fixes_applied > 0: @@ -651,10 +779,13 @@ def main() -> int: # Re-check translation gaps to show remaining issues translation_gaps = checker.check_translation_gaps() - # Check for orphaned strings in non-English locales + return translation_gaps + + +def _handle_orphaned_strings(checker: LocalizationChecker, run_cleanup_orphaned: bool) -> list[str]: + """Handle orphaned strings checking and cleanup.""" orphaned_strings = checker.check_orphaned_strings() - # Apply orphaned string cleanup if enabled if run_cleanup_orphaned and orphaned_strings: orphaned_fixes_applied = checker.apply_orphaned_string_cleanup() if orphaned_fixes_applied > 0: @@ -662,18 +793,23 @@ def main() -> int: # Re-check orphaned strings to show remaining issues orphaned_strings = checker.check_orphaned_strings() - # Normalize/sort all locale files for consistent ordering when not check-only - if not args.check_only: - for po_parser in checker.po_files.values(): - po_parser.write_back() + return orphaned_strings - # Report violations + +def _report_violations( + all_violations: list[str], + translation_gaps: list[str], + orphaned_strings: list[str], + run_autofix: bool, + run_cleanup_orphaned: bool, +) -> bool: + """Report all violations and return whether any were found.""" has_violations = False if all_violations: has_violations = True if run_autofix: - print(f"\nRemaining message localization violations:") + print("\nRemaining message localization violations:") else: print("Message localization violations:") for violation in all_violations: @@ -698,6 +834,52 @@ def main() -> int: for orphaned in orphaned_strings: print(orphaned) + return has_violations + + +def main() -> int: + """Main function.""" + parser = _create_argument_parser() + args = parser.parse_args() + + src_root: Path = args.path + locale_root: Path = args.locale_path + + # Determine which operations to run + run_autofix = not args.check_only and not args.no_autofix and not args.translation_gaps_only + run_fix_gaps = not args.check_only and not args.no_fix_gaps + run_cleanup_orphaned = not args.check_only and not args.no_cleanup_orphaned + + # Validate paths + if validation_error := _validate_paths(src_root, locale_root, args.translation_gaps_only): + return validation_error + + checker = LocalizationChecker(src_root, locale_root) + + # Show what operations will be performed + _show_operations(args.check_only, run_autofix, run_fix_gaps, run_cleanup_orphaned) + + all_violations = [] + + # Process source files if not translation-gaps-only mode + if not args.translation_gaps_only: + py_files = _get_python_files(src_root) + all_violations = _process_source_files(checker, py_files, run_autofix) + + # Handle translation gaps and orphaned strings + translation_gaps = _handle_translation_gaps(checker, run_fix_gaps) + orphaned_strings = _handle_orphaned_strings(checker, run_cleanup_orphaned) + + # Normalize/sort all locale files for consistent ordering when not check-only + if not args.check_only: + for po_parser in checker.po_files.values(): + po_parser.write_back() + + # Report violations + has_violations = _report_violations( + all_violations, translation_gaps, orphaned_strings, run_autofix, run_cleanup_orphaned + ) + if has_violations: return 1 @@ -710,5 +892,3 @@ def main() -> int: if __name__ == "__main__": raise SystemExit(main()) - - diff --git a/scripts/check_spdx.py b/scripts/check_spdx.py index c027fa6..8c670ec 100644 --- a/scripts/check_spdx.py +++ b/scripts/check_spdx.py @@ -35,38 +35,38 @@ '*.mo', } + def get_gitignore_spec(repo_root: str) -> pathspec.PathSpec: """Load .gitignore patterns.""" gitignore_file = os.path.join(repo_root, '.gitignore') if os.path.exists(gitignore_file): - with open(gitignore_file, 'r') as f: + with open(gitignore_file, 'r', encoding='utf-8') as f: return pathspec.PathSpec.from_lines( - pathspec.patterns.GitWildMatchPattern, - f.readlines() + pathspec.patterns.GitWildMatchPattern, f.readlines() ) return pathspec.PathSpec([]) + def should_skip(path: str, gitignore_spec: pathspec.PathSpec) -> bool: """Check if a file should be skipped.""" # Convert to relative path for gitignore matching rel_path = os.path.relpath(path, start=str(Path(__file__).parent.parent)) - + # Check gitignore patterns if gitignore_spec.match_file(rel_path): return True - + # Check our custom skip patterns parts = Path(path).parts - return any( - any(part.startswith(skip.rstrip('/')) for part in parts) - for skip in SKIP_PATTERNS - ) + return any(any(part.startswith(skip.rstrip('/')) for part in parts) for skip in SKIP_PATTERNS) + def has_spdx_header(content: str) -> bool: """Check if file already has an SPDX header.""" first_lines = content.split('\n')[:3] return any('SPDX-License-Identifier' in line for line in first_lines) + def find_python_files(start_dir: str, gitignore_spec: pathspec.PathSpec) -> List[str]: """Find all Python files in the directory tree.""" python_files = [] @@ -80,6 +80,7 @@ def find_python_files(start_dir: str, gitignore_spec: pathspec.PathSpec) -> List python_files.append(full_path) return python_files + def main(): """Main entry point.""" # Get repository root (assumes script is in scripts/ directory) @@ -110,5 +111,6 @@ def main(): print(f"Checked {len(python_files)} files; all have SPDX headers") return 0 + if __name__ == '__main__': sys.exit(main()) From c9ca4faba90ea5661f863ad8f826e6156541035b Mon Sep 17 00:00:00 2001 From: Sankalp Shrivastava Date: Wed, 17 Sep 2025 11:56:56 +0530 Subject: [PATCH 7/8] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- scripts/check_localization.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/scripts/check_localization.py b/scripts/check_localization.py index 0bd771a..349e888 100644 --- a/scripts/check_localization.py +++ b/scripts/check_localization.py @@ -40,8 +40,14 @@ @dataclass class PoEntry: - """Entry in a .po file.""" - + """ + Represents an entry in a gettext .po file. + + Attributes: + msgid (str): The message ID (original string in English). + msgstr (str): The translated string. + line_no (int): The line number in the .po file where this entry appears. + """ msgid: str msgstr: str line_no: int @@ -49,8 +55,15 @@ class PoEntry: @dataclass class LocalizationFix: - """Fix for a localization issue.""" - + """ + Represents a proposed fix for a localization issue. + + Attributes: + string_value (str): The string that requires localization. + source_file (str): The source file where the string was found. + line_no (int): The line number in the source file. + context (str): Description of where or how the string was found. + """ string_value: str source_file: str line_no: int From 724fdd91da3fa00f5aef3ebd2ce051b70c02828d Mon Sep 17 00:00:00 2001 From: shrivas Date: Wed, 17 Sep 2025 15:26:12 +0530 Subject: [PATCH 8/8] lint fix --- scripts/check_localization.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/check_localization.py b/scripts/check_localization.py index 349e888..b040160 100644 --- a/scripts/check_localization.py +++ b/scripts/check_localization.py @@ -48,6 +48,7 @@ class PoEntry: msgstr (str): The translated string. line_no (int): The line number in the .po file where this entry appears. """ + msgid: str msgstr: str line_no: int @@ -64,10 +65,11 @@ class LocalizationFix: line_no (int): The line number in the source file. context (str): Description of where or how the string was found. """ + string_value: str source_file: str line_no: int - context: str # Description of where the string was found + context: str class PoFileParser: