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

[styleCheck] --styleCheck:error inconsistent with --styleCheck:hint : doesn't respect package boundaries #10201

Closed
timotheecour opened this issue Jan 5, 2019 · 2 comments · Fixed by #19822
Labels

Comments

@timotheecour
Copy link
Member

--styleCheck:hint inconsistent with --styleCheck:error:
i would expect --styleCheck:hint to report exact same issues as --styleCheck:error but treating them as hints instead of errors, however:

proc main_bar() = discard
nim c --styleCheck:hint $timn_D/bugs/all/t0104.nim
Hint: used config file '/Users/timothee/git_clone/nim/Nim/config/nim.cfg' [Conf]
Hint: used config file '/Users/timothee/.config/nim/nim.cfg' [Conf]
Hint: used config file '/Users/timothee/git_clone/nim/timn/nim.cfg' [Conf]
Hint: used config file '/Users/timothee/git_clone/nim/Nim/config/config.nims' [Conf]
/Users/timothee/git_clone/nim/timn/bugs/all/t0104.nim(9, 6) Hint: name should be: 'mainBar' [Name]
  proc main_bar() = discard
       ^
Hint: operation successful (29804 lines compiled; 0.363 sec total; 25.137MiB peakmem; Debug Build) [SuccessX]
nim c --styleCheck:error $timn_D/bugs/all/t0104.nim
Hint: used config file '/Users/timothee/git_clone/nim/Nim/config/nim.cfg' [Conf]
Hint: used config file '/Users/timothee/.config/nim/nim.cfg' [Conf]
Hint: used config file '/Users/timothee/git_clone/nim/timn/nim.cfg' [Conf]
Hint: used config file '/Users/timothee/git_clone/nim/Nim/config/config.nims' [Conf]
../../../Nim/lib/system/ansi_c.nim(110, 6) Error: name should be: 'CSighandlerT'
@timotheecour
Copy link
Member Author

timotheecour commented Jan 5, 2019

upon further investigation (using the new flag --errormax:0), the root cause seems to be that --styleCheck:error doesn't respect package boundaries, but --styleCheck:hint respects package boundaries (only reports issues in same package).

so maybe the fix is to make --styleCheck:error respect package boundaries just like --styleCheck:hint

nim c --styleCheck:error --errormax:0 $timn_D/bugs/all/t0104.nim
Hint: used config file '/Users/timothee/git_clone/nim/Nim/config/nim.cfg' [Conf]
Hint: used config file '/Users/timothee/.config/nim/nim.cfg' [Conf]
Hint: used config file '/Users/timothee/git_clone/nim/timn/nim.cfg' [Conf]
Hint: used config file '/Users/timothee/git_clone/nim/Nim/config/config.nims' [Conf]
../../../Nim/lib/system/ansi_c.nim(110, 6) Error: name should be: 'CSighandlerT'
../../../Nim/lib/system/ansi_c.nim(110, 6) Error: name should be: 'CSighandlerT'
../../../../timotheecour/timconfig/dots_rename/config/nim/config.nims(95, 5) Error: name should be: 'timnD'
../../../Nim/lib/system/ansi_c.nim(110, 6) Error: name should be: 'CSighandlerT'
../../config.nims(9, 5) Error: name should be: 'gitCloneD'
../../config.nims(11, 5) Error: name should be: 'nimD'
Hint: system [Processing]
../../../Nim/lib/system/ansi_c.nim(110, 6) Error: name should be: 'CSighandlerT'
../../../Nim/lib/system/strmantle.nim(130, 5) Error: name should be: 'fracExponent'
../../../Nim/lib/system/strmantle.nim(131, 5) Error: name should be: 'expSign'
../../../Nim/lib/system/strmantle.nim(132, 5) Error: name should be: 'firstDigit'
../../../Nim/lib/system/strmantle.nim(133, 5) Error: name should be: 'hasSign'
../../../Nim/lib/system/strmantle.nim(209, 7) Error: name should be: 'realExponent'
../../../Nim/lib/system/strmantle.nim(210, 7) Error: name should be: 'expNegative'
../../../Nim/lib/system/strmantle.nim(211, 7) Error: name should be: 'absExponent'
Hint: t0104 [Processing]
t0104.nim(5, 6) Error: name should be: 'mainBar'
t0104.nim(5, 6) Hint: 'main_bar' is declared but not used [XDeclaredButNotUsed]

note

very relevant to this: #9854 (comment) see comment from @GULPF

Seems like the expected behavior to me. Why would Nim warn you about using deprecated procs in code that the package doesn't own?

note

but even then, --styleCheck:error --errormax:0 and --styleCheck:hint report different things on lib/system.nim, which also seems like another bug

@timotheecour timotheecour changed the title --styleCheck:hint inconsistent with --styleCheck:error --styleCheck:error inconsistent with --styleCheck:hint : doesn't respect package boundaries Jan 5, 2019
@timotheecour timotheecour changed the title --styleCheck:error inconsistent with --styleCheck:hint : doesn't respect package boundaries [styleCheck] --styleCheck:error inconsistent with --styleCheck:hint : doesn't respect package boundaries Jan 5, 2019
@Araq Araq added Tools and removed Nimpretty labels Jun 7, 2019
@timotheecour
Copy link
Member Author

timotheecour commented Nov 2, 2020

related: a --styleCheck:hint or --styleCheck:off in user config (only explicitly on command line works) won't help when building nim, because of this:

compiler/nim.cfg:19:3:  styleCheck:error

it's a pretty similar problem to #14272

a general fix to that kind of problem could be to attach to each flag X that's being set (via cmdline or config) a corresponding nimAssignedX, which can then be queried to honor user config, eg:

styleCheck:error
=>
@if not nimAssignedStyleCheck:
  styleCheck:error
@end 

(in this particular case, disabling --styleCheck is useful when recompiling nim with custom code imported that might not be styleCheck-proof, eg for debugging / customizing)

EDIT: workaround

styleCheck
in ~/.config/nim/config.nims add:

switch("undef", "nimHasStyleChecks")
switch("styleCheck", "off")

quantimnot added a commit to quantimnot/Nim that referenced this issue May 23, 2022
* Symbols from foreign packages are now ignored.
* Fixed `styleCheck` violations in `compiler` package.
* Added symbol ownership to custom annotation pragmas.
* Minor refactors to cleanup style check callsites.
* Minor internal documentation of reasons why a symbol isn't checked.

Style violations were fixed in the compiler after thet were exposed by
the changes. The compiler wouldn't compile otherwise.

Symbol ownership for custom pragma annotations is needed for checking
the annotation's style. A NPE was raised otherwise.

Fixes nim-lang#10201
See also nim-lang/RFCs#456
quantimnot added a commit to quantimnot/Nim that referenced this issue May 23, 2022
* Symbols from foreign packages are now ignored.
* Fixed `styleCheck` violations in `compiler` package.
* Added symbol ownership to custom annotation pragmas.
* Minor refactors to cleanup style check callsites.
* Minor internal documentation of reasons why a symbol isn't checked.

Style violations were fixed in the compiler after thet were exposed by
the changes. The compiler wouldn't compile otherwise.

Symbol ownership for custom pragma annotations is needed for checking
the annotation's style. A NPE was raised otherwise.

Fixes nim-lang#10201
See also nim-lang/RFCs#456
quantimnot added a commit to quantimnot/Nim that referenced this issue May 23, 2022
* Symbols from foreign packages are now ignored.
* Fix `styleCheck` violations in `compiler` package.
* Add symbol ownership to custom annotation pragmas.
* Minor refactors to cleanup style check callsites.
* Minor internal documentation of reasons why a symbol isn't checked.

Style violations were fixed in the compiler after thet were exposed by
the changes. The compiler wouldn't compile otherwise.

Symbol ownership for custom pragma annotations is needed for checking
the annotation's style. A NPE was raised otherwise.

Fixes nim-lang#10201
See also nim-lang/RFCs#456
quantimnot added a commit to quantimnot/Nim that referenced this issue May 23, 2022
* Symbols from foreign packages are now ignored.
* Fixed `styleCheck` violations in `compiler` package.
* Added symbol ownership to custom annotation pragmas.
* Minor refactors to cleanup style check callsites.
* Minor internal documentation of reasons why a symbol isn't checked.

Style violations were fixed in the compiler after thet were exposed by
the changes. The compiler wouldn't compile otherwise.

Symbol ownership for custom pragma annotations is needed for checking
the annotation's style. A NPE was raised otherwise.

Fixes nim-lang#10201
See also nim-lang/RFCs#456
quantimnot added a commit to quantimnot/Nim that referenced this issue May 30, 2022
* Symbols from foreign packages are now ignored.
* Fixed `styleCheck` violations in `compiler` package.
* Added symbol ownership to custom annotation pragmas.
* Minor refactors to cleanup style check callsites.
* Minor internal documentation of reasons why a symbol isn't checked.

Style violations were fixed in the compiler after thet were exposed by
the changes. The compiler wouldn't compile otherwise.

Symbol ownership for custom pragma annotations is needed for checking
the annotation's style. A NPE was raised otherwise.

Fixes nim-lang#10201
See also nim-lang/RFCs#456
quantimnot added a commit to quantimnot/Nim that referenced this issue Jun 4, 2022
* Symbols from foreign packages are now ignored.
* Fixed `styleCheck` violations in `compiler` package.
* Added symbol ownership to custom annotation pragmas.
* Minor refactors to cleanup style check callsites.
* Minor internal documentation of reasons why a symbol isn't checked.

Style violations were fixed in the compiler after thet were exposed by
the changes. The compiler wouldn't compile otherwise.

Symbol ownership for custom pragma annotations is needed for checking
the annotation's style. A NPE was raised otherwise.

Fixes nim-lang#10201
See also nim-lang/RFCs#456
quantimnot added a commit to quantimnot/Nim that referenced this issue Jul 8, 2022
* Symbols from foreign packages are now ignored.
* Fixed `styleCheck` violations in `compiler` package.
* Added symbol ownership to custom annotation pragmas.
* Minor refactors to cleanup style check callsites.
* Minor internal documentation of reasons why a symbol isn't checked.

Style violations were fixed in the compiler after thet were exposed by
the changes. The compiler wouldn't compile otherwise.

Symbol ownership for custom pragma annotations is needed for checking
the annotation's style. A NPE was raised otherwise.

Fixes nim-lang#10201
See also nim-lang/RFCs#456
quantimnot added a commit to quantimnot/Nim that referenced this issue Jul 13, 2022
* Symbols from foreign packages are now ignored.
* Fixed `styleCheck` violations in `compiler` package.
* Added symbol ownership to custom annotation pragmas.
* Minor refactors to cleanup style check callsites.
* Minor internal documentation of reasons why a symbol isn't checked.

Style violations were fixed in the compiler after thet were exposed by
the changes. The compiler wouldn't compile otherwise.

Symbol ownership for custom pragma annotations is needed for checking
the annotation's style. A NPE was raised otherwise.

Fixes nim-lang#10201
See also nim-lang/RFCs#456
Araq pushed a commit that referenced this issue Jul 14, 2022
* Change `styleCheck` to ignore foreign packages

* Symbols from foreign packages are now ignored.
* Fixed `styleCheck` violations in `compiler` package.
* Added symbol ownership to custom annotation pragmas.
* Minor refactors to cleanup style check callsites.
* Minor internal documentation of reasons why a symbol isn't checked.

Style violations were fixed in the compiler after thet were exposed by
the changes. The compiler wouldn't compile otherwise.

Symbol ownership for custom pragma annotations is needed for checking
the annotation's style. A NPE was raised otherwise.

Fixes #10201
See also nim-lang/RFCs#456

* Fix a misunderstanding about excluding field style checks

I had refactored the callsites of `styleCheckUse` to apply the DRY
principle, but I misunderstood the field access handling in a template
as a general case. This corrects it.

* Fix some `styleCheck` violations in `compiler/evalffi`

The violations were exposed in CI when the compiler was built with
libffi.

* Removed some uneeded transitionary code

* Add changelog entry

Co-authored-by: quantimnot <[email protected]>
FedericoCeratto pushed a commit to FedericoCeratto/Nim that referenced this issue Jul 30, 2022
* Change `styleCheck` to ignore foreign packages

* Symbols from foreign packages are now ignored.
* Fixed `styleCheck` violations in `compiler` package.
* Added symbol ownership to custom annotation pragmas.
* Minor refactors to cleanup style check callsites.
* Minor internal documentation of reasons why a symbol isn't checked.

Style violations were fixed in the compiler after thet were exposed by
the changes. The compiler wouldn't compile otherwise.

Symbol ownership for custom pragma annotations is needed for checking
the annotation's style. A NPE was raised otherwise.

Fixes nim-lang#10201
See also nim-lang/RFCs#456

* Fix a misunderstanding about excluding field style checks

I had refactored the callsites of `styleCheckUse` to apply the DRY
principle, but I misunderstood the field access handling in a template
as a general case. This corrects it.

* Fix some `styleCheck` violations in `compiler/evalffi`

The violations were exposed in CI when the compiler was built with
libffi.

* Removed some uneeded transitionary code

* Add changelog entry

Co-authored-by: quantimnot <[email protected]>
narimiran pushed a commit that referenced this issue Aug 2, 2022
* Change `styleCheck` to ignore foreign packages

* Symbols from foreign packages are now ignored.
* Fixed `styleCheck` violations in `compiler` package.
* Added symbol ownership to custom annotation pragmas.
* Minor refactors to cleanup style check callsites.
* Minor internal documentation of reasons why a symbol isn't checked.

Style violations were fixed in the compiler after thet were exposed by
the changes. The compiler wouldn't compile otherwise.

Symbol ownership for custom pragma annotations is needed for checking
the annotation's style. A NPE was raised otherwise.

Fixes #10201
See also nim-lang/RFCs#456

* Fix a misunderstanding about excluding field style checks

I had refactored the callsites of `styleCheckUse` to apply the DRY
principle, but I misunderstood the field access handling in a template
as a general case. This corrects it.

* Fix some `styleCheck` violations in `compiler/evalffi`

The violations were exposed in CI when the compiler was built with
libffi.

* Removed some uneeded transitionary code

* Add changelog entry

Co-authored-by: quantimnot <[email protected]>
(cherry picked from commit 800cb00)
capocasa pushed a commit to capocasa/Nim that referenced this issue Mar 31, 2023
* Change `styleCheck` to ignore foreign packages

* Symbols from foreign packages are now ignored.
* Fixed `styleCheck` violations in `compiler` package.
* Added symbol ownership to custom annotation pragmas.
* Minor refactors to cleanup style check callsites.
* Minor internal documentation of reasons why a symbol isn't checked.

Style violations were fixed in the compiler after thet were exposed by
the changes. The compiler wouldn't compile otherwise.

Symbol ownership for custom pragma annotations is needed for checking
the annotation's style. A NPE was raised otherwise.

Fixes nim-lang#10201
See also nim-lang/RFCs#456

* Fix a misunderstanding about excluding field style checks

I had refactored the callsites of `styleCheckUse` to apply the DRY
principle, but I misunderstood the field access handling in a template
as a general case. This corrects it.

* Fix some `styleCheck` violations in `compiler/evalffi`

The violations were exposed in CI when the compiler was built with
libffi.

* Removed some uneeded transitionary code

* Add changelog entry

Co-authored-by: quantimnot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants