Page MenuHomePhabricator

arc lint missed some error reported from the golint
Closed, WontfixPublic

Description

There is such a warning reported from golint : exported var ChangeState should have comment or be unexported
however it's not detected by arc lint, not sure what's the reason behind this

Steps to reproduce:
install the latest arcanist from github
create test.go with following content

package test

var (
	Test = 47
)

try golint test.go, output:test.go:4:2: exported var Test should have comment or be unexported
try arc lint, output: OKAY No lint warnings.

➜ ~ arc version
arcanist 89e8b48523844cc3eff8b775f8fae49e85f8fc22 (19 Aug 2016)
libphutil 237549280f08feb8852857b1d109dfb0aba345f0 (23 Aug 2016)

Event Timeline

rayzyar created this task.Aug 23 2016, 9:06 AM
chad added a subscriber: chad.Aug 23 2016, 1:57 PM

Please follow the instructions in Contributing Bug Reports. This is not a valid report without version information and step by step reproduction information.

rayzyar updated the task description. (Show Details)Aug 23 2016, 3:14 PM
rayzyar updated the task description. (Show Details)Aug 23 2016, 3:20 PM
In T11517#191349, @chad wrote:

Please follow the instructions in Contributing Bug Reports. This is not a valid report without version information and step by step reproduction information.

updated

rayzyar updated the task description. (Show Details)

@chad could u take a look again

chad added a comment.Aug 28 2016, 4:22 PM

See Planning for timelines on how we work. If this issue is urgent and blocking your company, see Support Resources for prioritization options.

Can you post your arclint file

michaeljs1990 added a comment.EditedAug 28 2016, 6:37 PM

You are going to have to post more information. I am able to correctly see the error you posted following these steps.

mkdir lolgo && cd lolgo
git init

cat >> .arclint << EOF
{
    "linters": {
        "golint": {
            "include": [
                "(\\.go$)"
            ],
            "type": "golint"
        }
     }
}
EOF

cat >> test.go<< EOF
package test

var (
	Test = 47
)
EOF
git add --all

now run arc lint and see the expected output.

14:32 $ arc lint
>>> Lint for test.go:


   Advice  (GOLINT) GOLINT
    Exported var Test should have comment or be unexported

               1 package test
               2 
               3 var (
    >>>        4     Test = 47
               5 )
               6 
 OKAY  No lint warnings.

I imagine you have something suppressing this warning in your configuration and it's not an issue with arcanist. This is using version c7bacac2b21ca01afa1dee0acf64df3ce047c28f of golint.

If you can provide steps to reproduce i'll fix this otherwise I think this one can be closed out as an environment issue.

michaeljs1990 triaged this task as Low priority.
rayzyar added a comment.EditedAug 29 2016, 3:14 AM

I've just tried to reproduce again, seems my steps cannot reproduce it.
I couldn't find out a good way to reproduce this bug.
Here is what I found :
when I change my existing code (removing the comment of an exported variable or method), arc lint at root git directory doesn't trigger the ADVICE
however arc lint xxx/xxx.go could trigger the lint ADVICE

And I tried to copy the same code to try to reproduce this, it works perfectly with arc lint at root git directory.
BTW, I am using some extension, https://github.com/zerodiff/traphic

Thank you a lot for looking into this

rayzyar added a comment.EditedAug 29 2016, 3:29 AM

I've just tried to reproduce again, seems my steps cannot reproduce it.
I couldn't find out a good way to reproduce this bug.
Here is what I found :
when I change my existing code (removing the comment of an exported variable or method), arc lint at root git directory doesn't trigger the ADVICE
however arc lint xxx/xxx.go could trigger the lint ADVICE
And I tried to copy the same code to try to reproduce this, it works perfectly with arc lint at root git directory.
BTW, I am using some extension, https://github.com/zerodiff/traphic
Thank you a lot for looking into this

I'll try again without loading that extension
update:
seems traphic is not affecting this

seems this is due to I didn't add the --lintall option,
the change is one line removal (existing file at the base commit)
thus it's considered as not concerned, but it is concerned actually because I didn't remove the entire function

michaeljs1990 removed michaeljs1990 as the assignee of this task.Nov 16 2016, 11:08 PM
epriestley closed this task as Wontfix.Sep 27 2018, 2:26 PM
epriestley claimed this task.
epriestley added a subscriber: epriestley.

This doesn't seem actionable and is likely mooted by T13098.