Skip to content

Commit

Permalink
fix(checks): do not assume all format checks have a valid regexp
Browse files Browse the repository at this point in the history
While doing changes here, the type annotations were improved for check
tests.

Fixes WEBLATE-15ZT
  • Loading branch information
nijel committed Nov 8, 2024
1 parent b0d42ea commit d802675
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 26 deletions.
8 changes: 0 additions & 8 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -332,14 +332,6 @@ plugins = [
"mypy_drf_plugin.main"
]

[[tool.mypy.overrides]]
# TODO: remove this once core can be validated with mypy
ignore_errors = true
module = [
"weblate.*.tests",
"weblate.*.tests.*"
]

[[tool.mypy.overrides]]
disallow_untyped_defs = true
ignore_missing_imports = true
Expand Down
7 changes: 5 additions & 2 deletions weblate/checks/format.py
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ def extract_string_python_brace(match: re.Match) -> str:
class BaseFormatCheck(TargetCheck):
"""Base class for format string checks."""

regexp: Pattern[str]
regexp: Pattern[str] | None = None
plural_parameter_regexp: Pattern[str] | None = None
default_disabled = True
normalize_remove: set[str] = set()
Expand All @@ -357,7 +357,6 @@ def check_generator(
# Use plural as source in case singular misses format string and plural has it
if (
len(sources) > 1
and self.regexp
and not self.extract_matches(sources[0])
and self.extract_matches(sources[1])
):
Expand Down Expand Up @@ -414,6 +413,8 @@ def extract_string(self, match: re.Match) -> str:
return extract_string_simple(match)

def extract_matches(self, string: str) -> list[str]:
if self.regexp is None:
return []
return [
self.cleanup_string(self.extract_string(match))
for match in self.regexp.finditer(string)
Expand Down Expand Up @@ -469,6 +470,8 @@ def check_single(self, source: str, target: str, unit: Unit) -> bool:
def check_highlight(self, source: str, unit: Unit):
if self.should_skip(unit):
return
if self.regexp is None:
return
match_objects = self.regexp.finditer(source)
for match in match_objects:
yield (match.start(), match.end(), match.group())
Expand Down
20 changes: 10 additions & 10 deletions weblate/checks/tests/test_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from weblate.lang.models import Language, Plural

if TYPE_CHECKING:
from weblate.checks.base import Check
from weblate.checks.base import BaseCheck


class MockLanguage(Language):
Expand Down Expand Up @@ -92,14 +92,14 @@ class MockUnit:

def __init__(
self,
id_hash=None,
flags="",
code="cs",
source="",
note="",
is_source=None,
target="",
context="",
id_hash: str | None = None,
flags: str | Flags = "",
code: str = "cs",
source: str | list[str] = "",
note: str = "",
is_source: bool | None = None,
target: str | list[str] = "",
context: str = "",
) -> None:
if id_hash is None:
id_hash = random.randint(0, 65536) # noqa: S311
Expand Down Expand Up @@ -147,7 +147,7 @@ def source_string(self):
class CheckTestCase(SimpleTestCase):
"""Generic test, also serves for testing base class."""

check: None | Check = None
check: None | BaseCheck = None
default_lang = "cs"

def setUp(self) -> None:
Expand Down
2 changes: 1 addition & 1 deletion weblate/checks/tests/test_fluent_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def __init__(
source: str,
target: str = "",
fluent_type: str | None = None,
unit_id: str | None = None,
unit_id: str = "",
is_source: bool = False,
) -> None:
self._fluent_type = fluent_type
Expand Down
21 changes: 16 additions & 5 deletions weblate/checks/tests/test_icu_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,22 @@

"""Tests for ICU MessageFormat checks."""

from __future__ import annotations

from weblate.checks.icu import ICUMessageFormatCheck, ICUSourceCheck
from weblate.checks.tests.test_checks import CheckTestCase, MockUnit


class ICUMessageFormatCheckTest(CheckTestCase):
check = ICUMessageFormatCheck()

id_hash = "icu_message_format"
flag = "icu-message-format"
flags = None
id_hash: str = "icu_message_format"
flag: str = "icu-message-format"
flags: None | str = None

def get_mock(self, source=None, flags=None):
def get_mock(
self, source: str | list[str] | None = None, flags: str | None = None
) -> MockUnit:
if not flags and self.flags:
flags = self.flags
elif flags and self.flags:
Expand All @@ -24,7 +28,10 @@ def get_mock(self, source=None, flags=None):
flags = f"{self.flag}, icu-flags:{flags}" if flags else self.flag

return MockUnit(
self.id_hash, flags=flags, source=source, is_source=source is not None
self.id_hash,
flags=flags,
source=source if source is not None else "",
is_source=source is not None,
)

def test_plain(self) -> None:
Expand Down Expand Up @@ -276,6 +283,10 @@ def test_check_no_highlight(self) -> None:

self.assertListEqual(highlights, [])

def test_check_target_plural(self) -> None:
unit = self.get_mock(["{count} apple", "{count} apples"])
self.assertFalse(self.check.check_target_unit(unit.sources, unit.sources, unit))


# This is a sub-class of our existing test set because this format is an extension
# of the other format and it should handle all existing syntax properly.
Expand Down

0 comments on commit d802675

Please sign in to comment.