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

feat(Tactic/Linter): lint unwanted unicode #16215

Open
wants to merge 55 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 51 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
3d6e8b0
Adds first idea for text-based linter that flags unicode outside a wh…
adomasbaliuka Aug 28, 2024
0f95a33
Adds comments to definitions
adomasbaliuka Aug 28, 2024
e6352fe
Update linter warning message
adomasbaliuka Aug 28, 2024
d5943cc
Implements some reviewer comments
adomasbaliuka Aug 28, 2024
74e37a7
Updates whitelist
adomasbaliuka Aug 28, 2024
aefad15
Adds missing docstring
adomasbaliuka Aug 28, 2024
62cc53d
Splits up sections of whitelist instead of doing in-line comments in …
adomasbaliuka Aug 28, 2024
9d36e1d
Adds unicode value (in hex) as part of linter warning
adomasbaliuka Aug 29, 2024
9e51102
Renames and fixes function printCodepointHex
adomasbaliuka Aug 29, 2024
be9d70f
Fix $ vs <| preference as some linter wants
adomasbaliuka Aug 29, 2024
1472252
Merge remote-tracking branch 'origin/master' into adomas_unicodeLinter
adomasbaliuka Aug 29, 2024
830171e
Moves test for parse?_errorContext to test/LintStyle.lean
adomasbaliuka Aug 29, 2024
56fb85f
merge master
adomasbaliuka Aug 29, 2024
f5d463d
Removes U+200C (Zero Width Non-Joiner ZWNJ) from unicode whitelist
adomasbaliuka Aug 29, 2024
606bc57
Removes U+FE0F (Variation Selector-16 VS16) from unicode whitelist
adomasbaliuka Aug 29, 2024
507a8b4
Makes whitelists sections into Array Char. Removes some more weird ch…
adomasbaliuka Aug 29, 2024
cbe641d
Fixes comment
adomasbaliuka Aug 29, 2024
7ff6eee
Merge remote-tracking branch 'origin/master' into adomas_unicodeLinter
adomasbaliuka Aug 30, 2024
3d97a89
Fixes docstring
adomasbaliuka Sep 1, 2024
ac721ca
add logic for unicode variant selectors
joneugster Aug 31, 2024
3a48f1f
fixes
joneugster Aug 31, 2024
ddd080d
Completely reworks unicode linter
adomasbaliuka Sep 3, 2024
e71bf1c
Fixes issue where endPos was given to String.next
adomasbaliuka Sep 4, 2024
4dcf120
Removes test-like thing that was checking effect of selectors.
adomasbaliuka Sep 4, 2024
c6c16cf
Adds test. Improves error message (missing selector vs wrong selector)
adomasbaliuka Sep 4, 2024
905d317
Merge remote-tracking branch 'origin/master' into adomas_unicodeLinter
joneugster Sep 4, 2024
4998828
updating iteration logic
joneugster Sep 4, 2024
b7bffc9
run mk_all
joneugster Sep 4, 2024
1359bbf
fix imports
joneugster Sep 4, 2024
700ca64
cleanup and fixes
joneugster Sep 4, 2024
721ab4c
remove unused import
joneugster Sep 4, 2024
e997a10
update test
joneugster Sep 4, 2024
30f87b0
fix ASCII range
joneugster Sep 4, 2024
ba46390
Minor changes
adomasbaliuka Sep 19, 2024
041a588
Removes comment about linefeed being last character
adomasbaliuka Sep 19, 2024
bf2df64
Adds test that special char lists are disjoint
adomasbaliuka Sep 19, 2024
2182aa2
Remove stray character c from List.
adomasbaliuka Sep 19, 2024
e5d7b19
Merge remote-tracking branch 'origin/master' into adomas_unicodeLinter
adomasbaliuka Sep 19, 2024
0508a0a
Implements parsing back error messages for unicodeVariant
adomasbaliuka Sep 23, 2024
3dd2565
cleanup
joneugster Sep 23, 2024
274a2dc
cleanup
joneugster Sep 23, 2024
c068d8c
add logic for autofix
joneugster Sep 23, 2024
3239f31
clean up auto-fix
joneugster Sep 24, 2024
333c0da
fix
joneugster Sep 24, 2024
5cf7701
apply suggestions from 'lake exe lint-style --fix'
joneugster Sep 24, 2024
fbfaf02
fix bad whitespace
joneugster Sep 24, 2024
c416e9a
Merge branch 'eugster/bad-whitespace-2409' into adomas_unicodeLinter
joneugster Sep 24, 2024
761f6cf
revert erronous fixes
joneugster Sep 24, 2024
35cf638
fix
joneugster Sep 24, 2024
2cb92f1
apply suggestions from 'lake exe lint-style --fix'
joneugster Sep 24, 2024
56fbf50
Merge remote-tracking branch 'origin/master' into adomas_unicodeLinter
joneugster Sep 24, 2024
0294d51
auto-fix for non-breaking-spaces
joneugster Sep 24, 2024
d48386a
fixes by 'lake exe lint-style --fix'
joneugster Sep 24, 2024
47a8b55
move unicode linter declarations to their own file
joneugster Sep 25, 2024
063b3b2
Merge remote-tracking branch 'origin/master' into adomas_unicodeLinter
joneugster Sep 25, 2024
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
8 changes: 4 additions & 4 deletions Mathlib/GroupTheory/GroupExtension/Defs.lean
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ such as splittings and equivalences.

```text
For multiplicative groups:
E ↘
↗ E ↘
1 → N ↓ G → 1
E' ↗︎️
↘ E' ↗

For additive groups:
E ↘
↗ E ↘
0 → N ↓ G → 0
E' ↗︎️
Comment on lines -25 to -32
Copy link
Member

Choose a reason for hiding this comment

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

I think these are genuinely intended to be non-breaking; the right solution is to put this in a code block.

Copy link
Collaborator

@joneugster joneugster Sep 25, 2024

Choose a reason for hiding this comment

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

Sorry these are about the text-variant-selector of the arrows characters, i.e. there were invisible \uFE0E following the diagonal arrows to prevent them form being displayed as emojis.

There are no spaces changed here.

If they are supposed to have the variant-selector (which is a fair choice) then, they can be added to the list of text-symbols defined here and the variant-selectors will be fixed automatically by lake exe lint-style --fix

↘ E' ↗
```

- `(Add?)GroupExtension.Splitting S`: structure for splittings of a group extension `S` of `G` by
Expand Down
366 changes: 361 additions & 5 deletions Mathlib/Tactic/Linter/TextBased.lean

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion Mathlib/Tactic/Widget/Calc.lean
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def suggestSteps (pos : Array Lean.SubExpr.GoalsLocation) (goalType : Expr) (par
@[server_rpc_method]
def CalcPanel.rpc := mkSelectionPanelRPC suggestSteps
"Please select subterms."
"Calc 🔍"
"Calc 🔍"
Copy link
Member

Choose a reason for hiding this comment

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

These are also intended to be non-breaking; I think using the unicode escape would be appropriate here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

same here, there is no space change, but the magnifying glass received a emoji-variant-selector because its specified in the list of emojis.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For context, there are currently no non-breaking spaces in any parts of mathlib except in docstrings about condensed mathematics & cat. theory because Dagur's IDE automatically adds them after each closing `

And the dependency PRs fixing whitespace are mainly because Dagur's quite busy and commiting a lot :D


/-- The calc widget. -/
@[widget_module]
Expand Down
2 changes: 1 addition & 1 deletion Mathlib/Tactic/Widget/CongrM.lean
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def makeCongrMString (pos : Array Lean.SubExpr.GoalsLocation) (goalType : Expr)
@[server_rpc_method]
def CongrMSelectionPanel.rpc := mkSelectionPanelRPC makeCongrMString
"Use shift-click to select sub-expressions in the goal that should become holes in congrm."
"CongrM 🔍"
"CongrM 🔍"

/-- The congrm widget. -/
@[widget_module]
Expand Down
2 changes: 1 addition & 1 deletion Mathlib/Tactic/Widget/Conv.lean
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def insertEnter (locations : Array Lean.SubExpr.GoalsLocation) (goalType : Expr)
def ConvSelectionPanel.rpc :=
mkSelectionPanelRPC insertEnter
"Use shift-click to select one sub-expression in the goal that you want to zoom on."
"Conv 🔍" (onlyGoal := false) (onlyOne := true)
"Conv 🔍" (onlyGoal := false) (onlyOne := true)

/-- The conv widget. -/
@[widget_module]
Expand Down
2 changes: 1 addition & 1 deletion Mathlib/Tactic/Widget/GCongr.lean
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ return (res, res, none)
@[server_rpc_method]
def GCongrSelectionPanel.rpc := mkSelectionPanelRPC makeGCongrString
"Use shift-click to select sub-expressions in the goal that should become holes in gcongr."
"GCongr 🔍"
"GCongr 🔍"

/-- The gcongr widget. -/
@[widget_module]
Expand Down
2 changes: 1 addition & 1 deletion Mathlib/Topology/ContinuousFunction/Weierstrass.lean
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ theorem polynomialFunctions_closure_eq_top (a b : ℝ) :
rw [Subalgebra.topologicalClosure_comap_homeomorph _ W W' w] at p
-- and precomposing with an affine map takes polynomial functions to polynomial functions.
rw [polynomialFunctions.comap_compRightAlgHom_iccHomeoI] at p
-- 🎉
-- 🎉
exact p
· -- Otherwise, `b ≤ a`, and the interval is a subsingleton,
subsingleton [(Set.subsingleton_Icc_of_ge h).coe_sort]
Expand Down
52 changes: 52 additions & 0 deletions test/LintStyle.lean
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,55 @@ lemma foo' : True := trivial
-- TODO: add terms for the term form

end setOption

section unicodeLinter

open Mathlib.Linter.TextBased
open Mathlib.Linter.TextBased.unicodeLinter

-- test parsing back error message in `parse?_errorContext` for unicode errors
#guard let errContext : ErrorContext := {
error := .unwantedUnicode '\u1234', lineNumber := 4, path:="./MYFILE.lean"}
(parse?_errorContext <| outputMessage errContext .exceptionsFile) == some errContext

-- test parsing back error message in `parse?_errorContext` for variant selector errors

-- "missing" selector
#guard let errContext : ErrorContext := {
error := .unicodeVariant "\u1234A" UnicodeVariant.emoji ⟨6⟩,
lineNumber := 4, path:="./MYFILE.lean"}
(parse?_errorContext <| outputMessage errContext .exceptionsFile) == some errContext

-- "wrong" selector
#guard let errContext : ErrorContext := {
error := .unicodeVariant "\u1234\uFE0E" UnicodeVariant.emoji ⟨6⟩,
lineNumber := 4, path:="./MYFILE.lean"}
(parse?_errorContext <| outputMessage errContext .exceptionsFile) == some errContext

-- "unexpected" selector
#guard let errContext : ErrorContext := {
error := .unicodeVariant "\uFE0EA" none ⟨6⟩, lineNumber := 4, path:="./MYFILE.lean"}
(parse?_errorContext <| outputMessage errContext .exceptionsFile) == some errContext

-- make sure hard-coded lists of unicode characters are disjoint (for clarity and maintainability)

-- lists of special characters should not have allowed ASCII.
#guard List.all [
othersInMathlib,
withVSCodeAbbrev,
emojis,
nonEmojis
] fun arr ↦ arr.all (!ASCII.allowed ·)


-- The lists of special characters should be disjoint.
#guard othersInMathlib.toList ∩ withVSCodeAbbrev.toList = ∅
#guard othersInMathlib.toList ∩ emojis.toList = ∅
#guard othersInMathlib.toList ∩ nonEmojis.toList = ∅

#guard withVSCodeAbbrev.toList ∩ emojis.toList = ∅
#guard withVSCodeAbbrev.toList ∩ nonEmojis.toList = ∅

#guard emojis.toList ∩ nonEmojis.toList = ∅

end unicodeLinter
Loading