region at the start of a normal comment triggers error about endregion being missing

[rchl]

Environment data

• Language Server version: v2023.1.30
• OS and version: macOS
• Python version (& distribution if applicable, e.g. Anaconda): -

Code Snippet

            for diagnostic, candidate in diagnostics:
                # Checking against points is inclusive unlike checking whether region intersects another
                # region which is exclusive (at region end) and we want an inclusive behavior in this case.

Repro Steps

1. Have a normal comment in the file that starts with a "region" word

Expected behavior

No error about missing "endregion" pair

Actual behavior

Getting error about "endregion" missing

Logs

Not relevant

[heejaechang]

@debonte dup?

[debonte]

Not a dupe. This is really about multi-line comments that happen to have one line starting with "region".

@rchl, thanks for filing this.

[debonte]

I don't see a way to improve detection of #region comments that would help in this scenario without breaking more common region scenarios and also making Pylance behave significantly differently from other Python editing experiences (ex. PyCharm). For example:

1. Require some specific formatting (ex. #region: with a colon).
2. Assume that #region at the start of a line other than the first line in a multi-line comment is not intended to be a region.

I think the best solution is to provide a way to disable the unmatched region diagnostics. This seems especially desirable in Pyright which doesn't support region folding. At least Pylance users have folding support as an incentive to get their #region comments recognized correctly.

@erictraut, any concerns with adding a new DiagnosticRule for this? Maybe disabled by default in Pyright and on by default in Pylance? I think Rich added a mechanism for reportShadowedImports that makes that possible.

@rchl, would you be satisfied with this solution?

Workarounds in the meantime to silence a particular instance of this diagnostic:

1. Rejustify the text in the comment so "region" isn't at the start of a line.
2. Add an extra # before the line in question (## region).
3. #type: ignore, but that seems ridiculous in the midst of a multi-line comment/paragraph.

[erictraut]

This doesn't strike me as something that justifies a new diagnostic rule. If we were to add a new diagnostic rule, it should definitely not have a different default because pyright and pylance. We don't have any precedent for that. The defaults are determined by the "none", "basic" or "strict" rule sets.

My suggestion here is to make the parsing a bit more strict, along the lines of what you're suggesting above. Do you see any good reason why we'd want to interpret something as a # region comment if it is followed by something other than whitespace or a colon? For example, I would think that it's unlikely that # region blah blah blah was meant to be interpreted as a region demarkation comment.

[debonte]

It's actually quite common (maybe even the majority of cases) from what I've seen searching through GitHub.

• https://github.com/curtisjamesholt/BY-GEN-public/blob/006811b227316145eb0d195e4f1511b5f671dd17/__init__.py?q=%23region+language%3APython#L98
• https://github.com/apple/unityplugins/blob/648ce6f5254424ca17759a26f9056cdf7ddcbf2a/build.py?q=%23region+language%3APython#L115
• https://github.com/open-mmlab/mmskeleton/blob/b4c076baa9e02e69b5876c49fa7c509866d902c7/deprecated/origin_stgcn_repo/processor/io.py?q=%23region+language%3APython#L95
• https://github.com/turanszkij/WickedEngine/blob/30c47c83ccc84bf658c6b24aab149a1ecc81d032/Editor/Linux/Installer/Installer.py?q=%23region+language%3APython#L3

[debonte]

it should definitely not have a different default [between] pyright and pylance. We don't have any precedent for that.

I think this is true for reportShadowedImports? https://github.com/microsoft/pyrx/blob/de4a60e8f212bb665a337632bedb14fde86d3c2c/packages/pylance-internal/src/server.ts#L721

[erictraut]

It's actually quite common

Ah, that's unfortunate. Is a colon (or no following text) used in the majority of cases? Would it make sense to say "if you don't use a colon, we will not treat it as a region demarkation"? That would eliminate false positive errors at the expense of forcing some users to add colons if they want region demarkations to work. That sounds preferable to me than adding a switch that we need to maintain and that user need to toggle to eliminate false positive errors in what is otherwise perfectly legal comments that happen to contain the word "region".

I think this is true for reportShadowedImports?

Ah, I forgot about that. I think this is an unfortunate precedent. We receive frequent questions from users about how to get the same behaviors between pyright and pylance. The more we diverge, the harder this becomes. So I think we should maintain a high bar for any divergence. IMO, this doesn't meet that bar.

[erictraut]

Don't other languages (like TS or C#) support region demarkation comments? How do they deal with this case? Perhaps there's some precedent we should be following here.

[debonte]

Is a colon (or no following text) used in the majority of cases?

No. Searching on cs.github.com, I'm seeing about 1K with a colon, 1K with just whitespace, and 40+K with text and no colon.

Don't other languages (like TS or C#) support region demarkation comments?

C# doesn't use comments. It uses the #region and #endregion preprocessor directives.

TypeScript uses //#region and //#endregion comments. Looking at various issues related to this, it appears that they considered also supporting //region and //endregion, but they ended up requiring the #. Maybe because they ran into this issue? I didn't find a GitHub comment that explained this decision though.

[rchl]

I'm reading those comments and I'm wondering whether we're missing an elephant in the room. My case was about comment that starts with # region while all the examples that use it for the purpose of region folding that I'm seeing are using #region so without the space after #. Only considering the latter case for region folding should eliminate large amount of false positives, I would think.

[debonte]

@rchl, cs.github.com finds 6k files without a space (#region) and 47k files with a space (# region).

To double check, i browsed through the files that had spaces and the ones I saw all appeared to be defining regions; they weren't comments that happened to contain the word "region" as in your case.

[github-actions]

This issue has been closed automatically because it needs more information and has not had recent activity. If the issue still persists, please reopen with the information requested. Thanks.

[concretevitamin]

Is there any way to disable this rule? Quite annoying.

[bpartridge]

This also triggers on a commented-out line like # region.some_method_on_the_region when there is a variable named region.

At the very least there should be a setting to opt-out from region comment tracking. If you're doing geospatial work with entities called region, you're likely not using region/endregion anyways.

[aflag]

I think at very least the start shouldn't be trimmed here: https://github.com/microsoft/pyright/blob/9a53b9962c763191690e48e92322d853df37ed0d/packages/pyright-internal/src/analyzer/regions.ts#L57. I think having #region having a special meaning is probably not too bad, but having # region having a special meaning is very annoying when "region" is an important noun in your code base.

[rchiodo]

This issue has been fixed in prerelease version 2024.2.101, which we've just released. You can find the changelog here: CHANGELOG.md