From f7c60a1373acdae90ba2672cb1c2c5d4e3b846ae Mon Sep 17 00:00:00 2001 From: Kurt McKee Date: Mon, 20 Jul 2026 09:41:29 -0500 Subject: [PATCH 1/3] Add linting infrastructure --- .github/workflows/test.yaml | 12 ++++++ .pre-commit-config.yaml | 82 +++++++++++++++++++++++++++++++++++++ pyproject.toml | 18 +++++++- tox.ini | 12 +++++- 4 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 .pre-commit-config.yaml diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 1a031b9..e32b64f 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -48,6 +48,18 @@ jobs: run: | uv tool run --with=tox-uv --with=tox-gh tox --colored=yes + lint: + name: 'Lint' + runs-on: 'ubuntu-24.04' + steps: + - name: 'Checkout' + uses: 'actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0' # v7.0.0 + with: + persist-credentials: 'false' + + - name: 'Run linters' + uses: 'j178/prek-action@e98a699c41eb69ab013a45817a0406469a748f8d' # v2.0.5 + security: name: 'Security' runs-on: 'ubuntu-slim' diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..2d647c4 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,82 @@ +default_language_version: + python: "python3.14" + +exclude: "tests/sgml_input\\.html" + +repos: + - repo: "meta" + hooks: + - id: "check-hooks-apply" + - id: "check-useless-excludes" + + - repo: "https://github.com/pre-commit/pre-commit-hooks" + rev: "3e8a8703264a2f4a69428a0aa4dcb512790b2c8c" # frozen: v6.0.0 + hooks: + - id: "check-added-large-files" + - id: "check-merge-conflict" + - id: "check-yaml" + - id: "end-of-file-fixer" + - id: "mixed-line-ending" + args: + - "--fix=lf" + - id: "trailing-whitespace" + + - repo: "https://github.com/asottile/pyupgrade" + rev: "75992aaa40730136014f34227e0135f63fc951b4" # frozen: v3.21.2 + hooks: + - id: "pyupgrade" + name: "Enforce Python 3.10+ idioms" + args: ["--py310-plus"] + + - repo: "https://github.com/sirosen/slyp" + rev: "072ea633cef088f143be7c16499d1564485dfc0c" # frozen: 0.9.0 + hooks: + - id: "slyp" + + - repo: "https://github.com/psf/black-pre-commit-mirror" + rev: "4160603246a6b365d4a2af661c6d71b0a0f50478" # frozen: 26.5.1 + hooks: + - id: "black" + args: + - "--target-version=py314" + + - repo: "https://github.com/pycqa/isort" + rev: "dac090ce4d9ee313d086e2e89ab1acb8c2664fa1" # frozen: 9.0.0a3 + hooks: + - id: "isort" + + - repo: "https://github.com/pycqa/flake8" + rev: "c48217e1fc006c2dddd14df54e83b67da15de5cd" # frozen: 7.3.0 + hooks: + - id: "flake8" + additional_dependencies: + - "flake8-toml-config==1.0.0" + - "flake8-bugbear==25.11.29" + + - repo: "https://github.com/editorconfig-checker/editorconfig-checker" + rev: "6fa6539948897d1dceee75b6680d7e9688b5c9e9" # frozen: v3.8.0 + hooks: + - id: "editorconfig-checker" + + - repo: "https://github.com/python-jsonschema/check-jsonschema" + rev: "6b63472e72e1a91ed8a2f6d483790dfb644fa1d3" # frozen: 0.37.4 + hooks: + - id: "check-dependabot" + - id: "check-readthedocs" + + - repo: "https://github.com/rhysd/actionlint" + rev: "914e7df21a07ef503a81201c76d2b11c789d3fca" # frozen: v1.7.12 + hooks: + - id: "actionlint" + additional_dependencies: + - "github.com/wasilibs/go-shellcheck/cmd/shellcheck@v0.11.1" + + - repo: "https://github.com/kurtmckee/pre-commit-hooks" + rev: "44b638c826f1acd2ba2c5995db3156be60c5efea" # frozen: v1.0.0 + hooks: + - id: "verify-consistent-pyproject-toml-python-requirements" + + - repo: "https://github.com/zizmorcore/zizmor-pre-commit" + rev: "e3eebf65325ccc992422292cb7a4baee967cf815" # frozen: v1.26.1 + hooks: + - id: "zizmor" diff --git a/pyproject.toml b/pyproject.toml index a2067ae..bc6ac7b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,6 +48,23 @@ skip_covered = true fail_under = 75 +# flake8 +# ------ + +[tool.flake8] +max-line-length = 80 +extend-select = ["B950"] +extend-ignore = ["E203", "E501", "E701"] + + +# isort +# ----- + +[tool.isort] +profile = "black" +force_single_line = true + + # pytest # ------ @@ -59,4 +76,3 @@ filterwarnings = [ "error", ] strict = true - diff --git a/tox.ini b/tox.ini index 73033f2..d2454ef 100644 --- a/tox.ini +++ b/tox.ini @@ -7,7 +7,7 @@ envlist = build{, -minimum} docs labels = - update = update-{requirements} + update = update-{pre_commit, requirements} [testenv] package = wheel @@ -81,6 +81,16 @@ base_python = py3.14 recreate = true skip_install = true +[testenv:update-pre_commit] +description = Update pre-commit hooks +base = update_base +deps = + prek + upadup +commands = + prek auto-update --cooldown-days 20 --freeze + upadup + [testenv:update-requirements] description = Update requirements files base = update_base From 32248afe1743b03020f69b8c52b1d84bbfde13da Mon Sep 17 00:00:00 2001 From: Kurt McKee Date: Mon, 20 Jul 2026 09:43:36 -0500 Subject: [PATCH 2/3] Remove dead code --- src/feedparser/sgmllib/__init__.py | 55 ------------------------------ 1 file changed, 55 deletions(-) diff --git a/src/feedparser/sgmllib/__init__.py b/src/feedparser/sgmllib/__init__.py index 7b8d0ad..5a1bb82 100644 --- a/src/feedparser/sgmllib/__init__.py +++ b/src/feedparser/sgmllib/__init__.py @@ -454,58 +454,3 @@ def unknown_starttag(self, tag, attrs): pass def unknown_endtag(self, tag): pass def unknown_charref(self, ref): pass def unknown_entityref(self, ref): pass - - -class TestSGMLParser(SGMLParser): - - def __init__(self, verbose=0): - self.testdata = "" - SGMLParser.__init__(self, verbose) - - def handle_data(self, data): - self.testdata = self.testdata + data - if len(repr(self.testdata)) >= 70: - self.flush() - - def flush(self): - data = self.testdata - if data: - self.testdata = "" - print('data:', repr(data)) - - def handle_comment(self, data): - self.flush() - r = repr(data) - if len(r) > 68: - r = r[:32] + '...' + r[-32:] - print('comment:', r) - - def unknown_starttag(self, tag, attrs): - self.flush() - if not attrs: - print('start tag: <' + tag + '>') - else: - print('start tag: <' + tag, end=' ') - for name, value in attrs: - print(name + '=' + '"' + value + '"', end=' ') - print('>') - - def unknown_endtag(self, tag): - self.flush() - print('end tag: ') - - def unknown_entityref(self, ref): - self.flush() - print('*** unknown entity ref: &' + ref + ';') - - def unknown_charref(self, ref): - self.flush() - print('*** unknown char ref: &#' + ref + ';') - - def unknown_decl(self, data): - self.flush() - print('*** unknown decl: [' + data + ']') - - def close(self): - SGMLParser.close(self) - self.flush() From 2e10fa7f45962a8e92b521cef1aba268af0ff2e1 Mon Sep 17 00:00:00 2001 From: Kurt McKee Date: Mon, 20 Jul 2026 09:46:20 -0500 Subject: [PATCH 3/3] Run `prek run --all-files` --- docs/conf.py | 2 +- docs/copyright.rst | 1 - src/feedparser/sgmllib/__init__.py | 196 +++++++------- tests/test_sgmllib.py | 397 ++++++++++++++++++----------- 4 files changed, 355 insertions(+), 241 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 1782d43..f7f8525 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -5,7 +5,7 @@ # --------------------- # The suffix of source filenames. -source_suffix = {'.rst': 'restructuredtext'} +source_suffix = {".rst": "restructuredtext"} # The main toctree document. master_doc = "index" diff --git a/docs/copyright.rst b/docs/copyright.rst index 7d53709..236c756 100644 --- a/docs/copyright.rst +++ b/docs/copyright.rst @@ -16,4 +16,3 @@ Copyright © 1991-1995 Stichting Mathematisch Centrum. All rights reserved. ------- See :doc:`license` for complete license and permissions information. - diff --git a/src/feedparser/sgmllib/__init__.py b/src/feedparser/sgmllib/__init__.py index 5a1bb82..ade89ab 100644 --- a/src/feedparser/sgmllib/__init__.py +++ b/src/feedparser/sgmllib/__init__.py @@ -16,28 +16,32 @@ # Regular expressions used for parsing -interesting = re.compile('[&<]') -incomplete = re.compile('&([a-zA-Z][a-zA-Z0-9]*|#[0-9]*)?|' - '<([a-zA-Z][^<>]*|' - '/([a-zA-Z][^<>]*)?|' - '![^<>]*)?') - -entityref = re.compile('&([a-zA-Z][-.a-zA-Z0-9]*)[^a-zA-Z0-9]') -charref = re.compile('&#([0-9]+)[^0-9]') - -starttagopen = re.compile('<[>a-zA-Z]') -shorttagopen = re.compile('<[a-zA-Z][-.a-zA-Z0-9]*/') -shorttag = re.compile('<([a-zA-Z][-.a-zA-Z0-9]*)/([^/]*)/') -piclose = re.compile('>') -endbracket = re.compile('[<>]') -tagfind = re.compile('[a-zA-Z][-_.a-zA-Z0-9]*') +interesting = re.compile("[&<]") +incomplete = re.compile( + "&([a-zA-Z][a-zA-Z0-9]*|#[0-9]*)?|" + "<([a-zA-Z][^<>]*|" + "/([a-zA-Z][^<>]*)?|" + "![^<>]*)?" +) + +entityref = re.compile("&([a-zA-Z][-.a-zA-Z0-9]*)[^a-zA-Z0-9]") +charref = re.compile("&#([0-9]+)[^0-9]") + +starttagopen = re.compile("<[>a-zA-Z]") +shorttagopen = re.compile("<[a-zA-Z][-.a-zA-Z0-9]*/") +shorttag = re.compile("<([a-zA-Z][-.a-zA-Z0-9]*)/([^/]*)/") +piclose = re.compile(">") +endbracket = re.compile("[<>]") +tagfind = re.compile("[a-zA-Z][-_.a-zA-Z0-9]*") attrfind = re.compile( - r'\s*([a-zA-Z_][-:.a-zA-Z_0-9]*)(\s*=\s*' - r'(\'[^\']*\'|"[^"]*"|[][\-a-zA-Z0-9./,:;+*%?!&$\(\)_#=~\'"@]*))?') + r"\s*([a-zA-Z_][-:.a-zA-Z_0-9]*)(\s*=\s*" + r'(\'[^\']*\'|"[^"]*"|[][\-a-zA-Z0-9./,:;+*%?!&$\(\)_#=~\'"@]*))?' +) class SGMLParseError(RuntimeError): """Exception raised for all parse errors.""" + pass @@ -52,13 +56,12 @@ class SGMLParseError(RuntimeError): # chunks). Entity references are passed by calling # self.handle_entityref() with the entity reference as argument. + class SGMLParser(_markupbase.ParserBase): # Definition of entities -- derived classes may override - entity_or_charref = re.compile('&(?:' - '([a-zA-Z][-.a-zA-Z0-9]*)|#([0-9]+)' - ')(;?)') + entity_or_charref = re.compile("&(?:([a-zA-Z][-.a-zA-Z0-9]*)|#([0-9]+))(;?)") - def __init__(self, verbose=0): + def __init__(self, verbose=0) -> None: """Initialize and reset this instance.""" self.verbose = verbose self.reset() @@ -66,9 +69,9 @@ def __init__(self, verbose=0): def reset(self): """Reset this instance. Loses all unprocessed data.""" self.__starttag_text = None - self.rawdata = '' + self.rawdata = "" self.stack = [] - self.lasttag = '???' + self.lasttag = "???" self.nomoretags = 0 self.literal = 0 _markupbase.ParserBase.reset(self) @@ -118,49 +121,56 @@ def goahead(self, end): i = n break match = interesting.search(rawdata, i) - if match: j = match.start() - else: j = n + if match: + j = match.start() + else: + j = n if i < j: self.handle_data(rawdata[i:j]) i = j - if i == n: break - if rawdata[i] == '<': + if i == n: + break + if rawdata[i] == "<": if starttagopen.match(rawdata, i): if self.literal: self.handle_data(rawdata[i]) - i = i+1 + i = i + 1 continue k = self.parse_starttag(i) - if k < 0: break + if k < 0: + break i = k continue if rawdata.startswith(" (i + 1): self.handle_data("<") - i = i+1 + i = i + 1 else: # incomplete break continue if rawdata.startswith(" ]""" - self.check_events(["" % inside], [ - ("decl", inside), - ]) + self.check_events( + ["" % inside], + [ + ("decl", inside), + ], + ) def test_doctype_decl_external(self): inside = "DOCTYPE html PUBLIC '-//W3C//DTD HTML 4.01//EN'" - self.check_events("" % inside, [ - ("decl", inside), - ]) + self.check_events( + "" % inside, + [ + ("decl", inside), + ], + ) def test_underscore_in_attrname(self): # SF bug #436621 """Make sure attribute names with underscores are accepted""" - self.check_events("", [ - ("starttag", "a", [("has_under", "has_under"), - ("_under", "_under")]), - ]) + self.check_events( + "", + [ + ("starttag", "a", [("has_under", "has_under"), ("_under", "_under")]), + ], + ) def test_underscore_in_tagname(self): # SF bug #436621 """Make sure tag names with underscores are accepted""" - self.check_events("", [ - ("starttag", "has_under", []), - ("endtag", "has_under"), - ]) + self.check_events( + "", + [ + ("starttag", "has_under", []), + ("endtag", "has_under"), + ], + ) def test_quotes_in_unquoted_attrs(self): # SF bug #436621 """Be sure quotes in unquoted attributes are made part of the value""" - self.check_events("", [ - ("starttag", "a", [("href", "foo'bar\"baz")]), - ]) + self.check_events( + "", + [ + ("starttag", "a", [("href", "foo'bar\"baz")]), + ], + ) def test_xhtml_empty_tag(self): """Handling of XHTML-style empty start tags""" - self.check_events("
text", [ - ("starttag", "br", []), - ("data", "text"), - ("starttag", "i", []), - ("endtag", "i"), - ]) + self.check_events( + "
text", + [ + ("starttag", "br", []), + ("data", "text"), + ("starttag", "i", []), + ("endtag", "i"), + ], + ) def test_processing_instruction_only(self): - self.check_events("", [ - ("pi", "processing instruction"), - ]) + self.check_events( + "", + [ + ("pi", "processing instruction"), + ], + ) def test_bad_nesting(self): - self.check_events("", [ - ("starttag", "a", []), - ("starttag", "b", []), - ("endtag", "a"), - ("endtag", "b"), - ]) + self.check_events( + "", + [ + ("starttag", "a", []), + ("starttag", "b", []), + ("endtag", "a"), + ("endtag", "b"), + ], + ) def test_bare_ampersands(self): - self.check_events("this text & contains & ampersands &", [ - ("data", "this text & contains & ampersands &"), - ]) + self.check_events( + "this text & contains & ampersands &", + [ + ("data", "this text & contains & ampersands &"), + ], + ) def test_bare_pointy_brackets(self): - self.check_events("this < text > contains < bare>pointy< brackets", [ - ("data", "this < text > contains < bare>pointy< brackets"), - ]) + self.check_events( + "this < text > contains < bare>pointy< brackets", + [ + ("data", "this < text > contains < bare>pointy< brackets"), + ], + ) def test_attr_syntax(self): - output = [ - ("starttag", "a", [("b", "v"), ("c", "v"), ("d", "v"), ("e", "e")]) - ] + output = [("starttag", "a", [("b", "v"), ("c", "v"), ("d", "v"), ("e", "e")])] self.check_events("""""", output) self.check_events("""""", output) self.check_events("""""", output) self.check_events("""""", output) def test_attr_values(self): - self.check_events("""""", - [("starttag", "a", [("b", "xxx\n\txxx"), - ("c", "yyy\t\nyyy"), - ("d", "\txyz\n")]) - ]) - self.check_events("""""", [ - ("starttag", "a", [("b", ""), ("c", "")]), - ]) + self.check_events( + """""", + [ + ( + "starttag", + "a", + [("b", "xxx\n\txxx"), ("c", "yyy\t\nyyy"), ("d", "\txyz\n")], + ) + ], + ) + self.check_events( + """""", + [ + ("starttag", "a", [("b", ""), ("c", "")]), + ], + ) # URL construction stuff from RFC 1808: safe = "$-_.+" extra = "!*'()," reserved = ";/?:@&=" - url = "http://example.com:8080/path/to/file?%s%s%s" % ( - safe, extra, reserved) - self.check_events("""""" % url, [ - ("starttag", "e", [("a", url)]), - ]) + url = "http://example.com:8080/path/to/file?{}{}{}".format( + safe, extra, reserved + ) + self.check_events( + """""" % url, + [ + ("starttag", "e", [("a", url)]), + ], + ) # Regression test for SF patch #669683. - self.check_events("", [ - ("starttag", "e", [("a", "rgb(1,2,3)")]), - ]) + self.check_events( + "", + [ + ("starttag", "e", [("a", "rgb(1,2,3)")]), + ], + ) def test_attr_values_entities(self): """Substitution of entities and charrefs in attribute values""" # SF bug #1452246 - self.check_events("""""", - [("starttag", "a", [("b", "<"), - ("c", "<>"), - ("d", "<->"), - ("e", "< "), - ("f", "&xxx;"), - ("g", " !"), - ("h", "Ǵ"), - ("i", "x?a=b&c=d;"), - ("j", "*"), - ("k", "*"), - ])]) + [ + ( + "starttag", + "a", + [ + ("b", "<"), + ("c", "<>"), + ("d", "<->"), + ("e", "< "), + ("f", "&xxx;"), + ("g", " !"), + ("h", "Ǵ"), + ("i", "x?a=b&c=d;"), + ("j", "*"), + ("k", "*"), + ], + ) + ], + ) def test_convert_overrides(self): # This checks that the character and entity reference @@ -268,86 +321,119 @@ def test_convert_overrides(self): # attempt is made to really change what the parser accepts. # self.collector = HTMLEntityCollector - self.check_events(('foo' - '&foobar;*'), [ - ('entityref', 'convert', 'ldquo'), - ('charref', 'convert', 'x201d'), - ('starttag', 'a', [('title', '“test”')]), - ('data', 'foo'), - ('endtag', 'a'), - ('entityref', 'foobar'), - ('entityref', 'convert', 'foobar'), - ('charref', '42'), - ('charref', 'convert', '42'), - ('codepoint', 'convert', 42), - ]) + self.check_events( + 'foo&foobar;*', + [ + ("entityref", "convert", "ldquo"), + ("charref", "convert", "x201d"), + ("starttag", "a", [("title", "“test”")]), + ("data", "foo"), + ("endtag", "a"), + ("entityref", "foobar"), + ("entityref", "convert", "foobar"), + ("charref", "42"), + ("charref", "convert", "42"), + ("codepoint", "convert", 42), + ], + ) def test_attr_funky_names(self): - self.check_events("""""", [ - ("starttag", "a", [("a.b", "v"), ("c:d", "v"), ("e-f", "v")]), - ]) + self.check_events( + """""", + [ + ("starttag", "a", [("a.b", "v"), ("c:d", "v"), ("e-f", "v")]), + ], + ) def test_attr_value_ip6_url(self): # http://www.python.org/sf/853506 - self.check_events(("" - ""), [ - ("starttag", "a", [("href", "http://[1080::8:800:200C:417A]/")]), - ("starttag", "a", [("href", "http://[1080::8:800:200C:417A]/")]), - ]) + self.check_events( + ( + "" + "" + ), + [ + ("starttag", "a", [("href", "http://[1080::8:800:200C:417A]/")]), + ("starttag", "a", [("href", "http://[1080::8:800:200C:417A]/")]), + ], + ) def test_weird_starttags(self): - self.check_events("", [ - ("starttag", "a", []), - ("starttag", "a", []), - ]) - self.check_events("", [ - ("endtag", "a"), - ("starttag", "a", []), - ]) + self.check_events( + "", + [ + ("starttag", "a", []), + ("starttag", "a", []), + ], + ) + self.check_events( + "", + [ + ("endtag", "a"), + ("starttag", "a", []), + ], + ) def test_declaration_junk_chars(self): self.check_parse_error("") def test_get_starttag_text(self): s = """""" - self.check_events(s, [ - ("starttag", "foobar", [("one", "1"), ("two", "2")]), - ]) + self.check_events( + s, + [ + ("starttag", "foobar", [("one", "1"), ("two", "2")]), + ], + ) def test_cdata_content(self): - s = (" ¬-an-entity-ref; " - " ") + s = ( + " ¬-an-entity-ref; " + " " + ) self.collector = CDATAEventCollector - self.check_events(s, [ - ("starttag", "cdata", []), - ("data", " ¬-an-entity-ref; "), - ("endtag", "cdata"), - ("starttag", "notcdata", []), - ("data", " "), - ("comment", " comment "), - ("data", " "), - ("endtag", "notcdata"), - ]) + self.check_events( + s, + [ + ("starttag", "cdata", []), + ("data", " ¬-an-entity-ref; "), + ("endtag", "cdata"), + ("starttag", "notcdata", []), + ("data", " "), + ("comment", " comment "), + ("data", " "), + ("endtag", "notcdata"), + ], + ) s = """ """ - self.check_events(s, [ - ("starttag", "cdata", []), - ("data", " "), - ("endtag", "cdata"), - ]) + self.check_events( + s, + [ + ("starttag", "cdata", []), + ("data", " "), + ("endtag", "cdata"), + ], + ) def test_illegal_declarations(self): s = 'abcdef' - self.check_events(s, [ - ("data", "abc"), - ("unknown decl", 'spacer type="block" height="25"'), - ("data", "def"), - ]) + self.check_events( + s, + [ + ("data", "abc"), + ("unknown decl", 'spacer type="block" height="25"'), + ("data", "def"), + ], + ) def test_enumerated_attr_type(self): s = "]>" - self.check_events(s, [ - ('decl', 'DOCTYPE doc []'), - ]) + self.check_events( + s, + [ + ("decl", "DOCTYPE doc []"), + ], + ) def test_read_chunks(self): # SF bug #1541697, this caused sgml parser to hang @@ -355,8 +441,8 @@ def test_read_chunks(self): CHUNK = 1024 # increasing this to 8212 makes the problem go away fp = sgmllib.SGMLParser() - path = pathlib.Path(__file__).parent / 'sgml_input.html' - with path.open(encoding='ISO-8859-1') as f: + path = pathlib.Path(__file__).parent / "sgml_input.html" + with path.open(encoding="ISO-8859-1") as f: while 1: data = f.read(CHUNK) fp.feed(data) @@ -366,11 +452,20 @@ def test_read_chunks(self): def test_only_decode_ascii(self): # SF bug #1651995, make sure non-ascii character references are not decoded s = '' - self.check_events(s, [ - ('starttag', 'signs', - [('exclamation', '!'), ('copyright', '©'), - ('quoteleft', '‘')]), - ]) + self.check_events( + s, + [ + ( + "starttag", + "signs", + [ + ("exclamation", "!"), + ("copyright", "©"), + ("quoteleft", "‘"), + ], + ), + ], + ) # XXX These tests have been disabled by prefixing their names with # an underscore. The first two exercise outstanding bugs in the