Skip to content
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 by Bors] - feat: dupNamespace as a syntax linter #11154

Closed
wants to merge 24 commits into from

Conversation

adomani
Copy link
Collaborator

@adomani adomani commented Mar 4, 2024

An implementation of the dupNamespace from Std as a syntax linter. The linter emits a warning for declarations that have a repeated, consecutive namespace:

  • Nat.Nat.foo triggers the linter;
  • Nat.foo.Nat does not trigger the linter.

Note. This linter will not detect duplication in namespaces of autogenerated declarations (other than the one whose declId is present in the source declaration).

Zulip discussion

The linter found

private theorem Seminorm.Seminorm.isLUB_sSup ...

that is skipped by the environment dupNamespace linter.


A note on the extra import: it is only used to get Array.attach, which allows to write

def getIds : Syntax → Array Syntax
  | stx@(.node _ _ args) =>
    ((args.attach.map fun ⟨a, _⟩ => getIds a).foldl (· ++ ·) #[stx]).filter (·.getKind == ``declId)
  | _ => default

and not have to mark it as partial.

Open in Gitpod

@adomani adomani added awaiting-review The author would like community review of the PR t-meta Tactics, attributes or user commands labels Mar 4, 2024
mathlib-bors bot pushed a commit that referenced this pull request Mar 5, 2024
…11161)

A private duplicated namespace found by the linter in #11154.
mathlib-bors bot pushed a commit that referenced this pull request Mar 5, 2024
…11161)

A private duplicated namespace found by the linter in #11154.
@alexjbest
Copy link
Member

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit de74382.
There were no significant changes against commit 39f3b00.

kbuzzard pushed a commit that referenced this pull request Mar 12, 2024
…11161)

A private duplicated namespace found by the linter in #11154.
dagurtomas pushed a commit that referenced this pull request Mar 22, 2024
…11161)

A private duplicated namespace found by the linter in #11154.
utensil pushed a commit that referenced this pull request Mar 26, 2024
…11161)

A private duplicated namespace found by the linter in #11154.
Copy link
Collaborator

@YaelDillies YaelDillies left a comment

Choose a reason for hiding this comment

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

maintainer merge

Mathlib/Tactic/Lint.lean Outdated Show resolved Hide resolved
Copy link

🚀 Pull request has been placed on the maintainer queue by YaelDillies.

1 similar comment
Copy link

🚀 Pull request has been placed on the maintainer queue by YaelDillies.

test/Simps.lean Outdated Show resolved Hide resolved
@mattrobball
Copy link
Collaborator

It would be nice to have tests for the behavior under exporting and aliasing also.

@adomani
Copy link
Collaborator Author

adomani commented Mar 28, 2024

!bench

@adomani adomani force-pushed the adomani/dupNamespace_syntax branch from 9c29121 to a778c08 Compare March 28, 2024 18:33
@adomani
Copy link
Collaborator Author

adomani commented Mar 28, 2024

I added checking for alias: it was slightly awkward, since the "name" node for alias is ident, while the one for theorem/lemma is declId. I have not checked what happens with export, though.

@leanprover-bot

This comment was marked as resolved.

@adomani
Copy link
Collaborator Author

adomani commented Mar 28, 2024

Here are the benchmark results for commit 9c29121. The entire run failed. Found no significant differences.

The bench should happen at #11747 reported no significant changes.

@mattrobball
Copy link
Collaborator

Great!

Can you write a test for naming via export and comment it out with some explanation if it doesn't work?

bors delegate+

@mathlib-bors
Copy link

mathlib-bors bot commented Mar 28, 2024

✌️ adomani can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@adomani
Copy link
Collaborator Author

adomani commented Mar 28, 2024

Matt, thanks! I added support for export and a test. Assuming that CI is happy with the change, I will merge!

@adomani
Copy link
Collaborator Author

adomani commented Mar 29, 2024

bors r+

mathlib-bors bot pushed a commit that referenced this pull request Mar 29, 2024
An implementation of the `dupNamespace` from `Std` as a syntax linter.  The linter emits a warning for declarations that have a repeated, consecutive namespace:
* `Nat.Nat.foo` triggers the linter;
* `Nat.foo.Nat` does *not* trigger the linter.

*Note.* This linter will not detect duplication in namespaces of autogenerated declarations (other than the one whose `declId` is present in the source declaration).


[Zulip discussion](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/.60dupNamespace.60.20linter)

The linter found
```lean
private theorem Seminorm.Seminorm.isLUB_sSup ...
```
that is skipped by the environment `dupNamespace` linter.
@mathlib-bors
Copy link

mathlib-bors bot commented Mar 29, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat: dupNamespace as a syntax linter [Merged by Bors] - feat: dupNamespace as a syntax linter Mar 29, 2024
@mathlib-bors mathlib-bors bot closed this Mar 29, 2024
@mathlib-bors mathlib-bors bot deleted the adomani/dupNamespace_syntax branch March 29, 2024 06:00
Louddy pushed a commit that referenced this pull request Apr 15, 2024
…11161)

A private duplicated namespace found by the linter in #11154.
Louddy pushed a commit that referenced this pull request Apr 15, 2024
An implementation of the `dupNamespace` from `Std` as a syntax linter.  The linter emits a warning for declarations that have a repeated, consecutive namespace:
* `Nat.Nat.foo` triggers the linter;
* `Nat.foo.Nat` does *not* trigger the linter.

*Note.* This linter will not detect duplication in namespaces of autogenerated declarations (other than the one whose `declId` is present in the source declaration).


[Zulip discussion](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/.60dupNamespace.60.20linter)

The linter found
```lean
private theorem Seminorm.Seminorm.isLUB_sSup ...
```
that is skipped by the environment `dupNamespace` linter.
atarnoam pushed a commit that referenced this pull request Apr 16, 2024
An implementation of the `dupNamespace` from `Std` as a syntax linter.  The linter emits a warning for declarations that have a repeated, consecutive namespace:
* `Nat.Nat.foo` triggers the linter;
* `Nat.foo.Nat` does *not* trigger the linter.

*Note.* This linter will not detect duplication in namespaces of autogenerated declarations (other than the one whose `declId` is present in the source declaration).


[Zulip discussion](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/.60dupNamespace.60.20linter)

The linter found
```lean
private theorem Seminorm.Seminorm.isLUB_sSup ...
```
that is skipped by the environment `dupNamespace` linter.
uniwuni pushed a commit that referenced this pull request Apr 19, 2024
An implementation of the `dupNamespace` from `Std` as a syntax linter.  The linter emits a warning for declarations that have a repeated, consecutive namespace:
* `Nat.Nat.foo` triggers the linter;
* `Nat.foo.Nat` does *not* trigger the linter.

*Note.* This linter will not detect duplication in namespaces of autogenerated declarations (other than the one whose `declId` is present in the source declaration).


[Zulip discussion](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/.60dupNamespace.60.20linter)

The linter found
```lean
private theorem Seminorm.Seminorm.isLUB_sSup ...
```
that is skipped by the environment `dupNamespace` linter.
callesonne pushed a commit that referenced this pull request Apr 22, 2024
…11161)

A private duplicated namespace found by the linter in #11154.
callesonne pushed a commit that referenced this pull request Apr 22, 2024
An implementation of the `dupNamespace` from `Std` as a syntax linter.  The linter emits a warning for declarations that have a repeated, consecutive namespace:
* `Nat.Nat.foo` triggers the linter;
* `Nat.foo.Nat` does *not* trigger the linter.

*Note.* This linter will not detect duplication in namespaces of autogenerated declarations (other than the one whose `declId` is present in the source declaration).


[Zulip discussion](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/.60dupNamespace.60.20linter)

The linter found
```lean
private theorem Seminorm.Seminorm.isLUB_sSup ...
```
that is skipped by the environment `dupNamespace` linter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review The author would like community review of the PR maintainer-merge t-meta Tactics, attributes or user commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants