Skip to content

Add check for illegal Widnows names #1031

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

Merged
merged 1 commit into from
Apr 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .pre-commit-hooks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@
language: python
types: [text, executable]
stages: [commit, push, manual]
- id: check-illegal-windows-names
name: check illegal windows names
entry: Illegal windows filenames detected
language: fail
files: '(?i)(^|/)(CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9])(\.|/|$)'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a quick little test which loads this regex from the configuration and runs it against a few expected values?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asottile @ericfrederich I recommend checking for the : character as well (see my comment in #589 regarding :Zone.Identifier files).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jleclanche looks like this got merged without code checking for the : but no release has been made yet.

I just checked and it's definitely a problem on Windows. I created a file foo:bar on linux, commited, pushed, then pulled down the repo from Windows and got greeted with error: invalid path 'foo:bar'

I'll add a test for it and a new PR

- id: check-json
name: check json
description: checks json files for parseable syntax.
Expand Down
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ Checks for a common error of placing code before the docstring.
#### `check-executables-have-shebangs`
Checks that non-binary executables have a proper shebang.

#### `check-illegal-windows-names`
Check for files that cannot be created on Windows.

#### `check-json`
Attempts to load all json files to verify syntax.

Expand Down
46 changes: 46 additions & 0 deletions tests/check_illegal_windows_names_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
from __future__ import annotations

import os.path
import re

import pytest

from pre_commit_hooks.check_yaml import yaml


@pytest.fixture(scope='module')
def hook_re():
here = os.path.dirname(__file__)
with open(os.path.join(here, '..', '.pre-commit-hooks.yaml')) as f:
hook_defs = yaml.load(f)
hook, = (
hook
for hook in hook_defs
if hook['id'] == 'check-illegal-windows-names'
)
yield re.compile(hook['files'])


@pytest.mark.parametrize(
's',
(
pytest.param('aux.txt', id='with ext'),
pytest.param('aux', id='without ext'),
pytest.param('AuX.tXt', id='capitals'),
pytest.param('com7.dat', id='com with digit'),
),
)
def test_check_illegal_windows_names_matches(hook_re, s):
assert hook_re.search(s)


@pytest.mark.parametrize(
's',
(
pytest.param('README.md', id='standard file'),
pytest.param('foo.aux', id='as ext'),
pytest.param('com.dat', id='com without digit'),
),
)
def test_check_illegal_windows_names_does_not_match(hook_re, s):
assert hook_re.search(s) is None