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.

Sign up for GitHub

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

Jump to bottom

Add SwiftLint and Swift Maker #2463

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add SwiftLint and Swift Maker #2463

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e391590
Swift: Add swiftlint Maker
jandamm Feb 6, 2020
580a98f
Swift: Add swift (file) Maker
jandamm Feb 6, 2020
1fc9dbb
Do not use l: prefix for local variables
jandamm Feb 6, 2020
55e9321
Rename variable
jandamm Feb 7, 2020
2bf8ae5
Improve swiftlint error matching
jandamm Feb 26, 2020
2fcb9c3
Add swiftpm error without line or column
jandamm Mar 1, 2020
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
31 changes: 29 additions & 2 deletions autoload/neomake/makers/ft/swift.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,32 @@
" vim: ts=4 sw=4 et

function! neomake#makers#ft#swift#EnabledMakers() abort
let ret = ['swiftc']
if !empty(s:get_swiftpm_config())
return ['swiftpm']
let ret = ['swiftpm']
endif
return ['swiftc']
if s:get_swiftlint_config()
let ret = add(ret, 'swiftlint')
endif
return ret
endfunction

function! s:get_swiftpm_config() abort
return neomake#utils#FindGlobFile('Package.swift')
endfunction

function! s:get_swiftlint_config() abort
return executable('swiftlint') && !empty(neomake#utils#FindGlobFile('.swiftlint.yml'))
endfunction
Copy link
Member

Choose a reason for hiding this comment

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

Isn't there also such a thing as a global config?

I think it makes sense to not require a local config for linters to be enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SwiftLint officially does not support global configs. It does have a default configuration which I think many wouldn't want to be enabled for every file they open. E.g. it marks every variable shorter than two letters as an error.
The convention is having a config file in your project or provide it via an argument.

I think the best solution would be to search for a config file then run swiftlint, check for ~/.swiftlint.yml if it exists run swiftlintglobal (with config provided).

Copy link
Member

@blueyed blueyed Feb 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

But wouldn't people not liking swiftlint then just not install it?
I.e. if you have it it should be used - not only if you have some explicit config for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have it installed because some projects I work on use it, but others don't.
I cannot speak for everyone but I wouldn't want the default rules enabled and I'm not sure many are using a global config (there is a pretty old issue requesting this, though)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've had a look at swift.vim.
There swiftlint is also only enabled when a .swiftlint.yml is present or the user requests it.
https://github.com/keith/swift.vim/blob/master/syntax_checkers/swift/swiftlint.vim#L14

What is your opinion? I would leave the current implementation as is. The user can always opt in and run/enable swiftlint for himself.


function! s:get_swiftpm_base_maker() abort
let maker = {
\ 'exe': 'swift',
\ 'append_file': 0,
\ 'errorformat':
\ '%E%f:%l:%c: error: %m,' .
\ '%E%f:%l: error: %m,' .
\ '%E%f: error: %m,' .
\ '%W%f:%l:%c: warning: %m,' .
\ '%Z%\s%#^~%#,' .
\ '%-G%.%#',
Expand All @@ -41,6 +50,24 @@ function! neomake#makers#ft#swift#swiftpmtest() abort
return maker
endfunction

function! neomake#makers#ft#swift#swiftfile() abort
let maker = s:get_swiftpm_base_maker()
let maker.append_file = 1
return maker
endfunction

function! neomake#makers#ft#swift#swiftlint() abort
return {
\ 'args': ['lint', '--quiet'],
\ 'append_file': 1,
\ 'errorformat':
\ '%f:%l:%c: %trror: %m,' .
\ '%f:%l:%c: %tarning: %m,' .
\ '%f:%l: %trror: %m,' .
\ '%f:%l: %tarning: %m',
\ }
jandamm marked this conversation as resolved.
Show resolved Hide resolved
endfunction

function! neomake#makers#ft#swift#swiftc() abort
" `export SDKROOT="$(xcodebuild -version -sdk macosx Path)"`
return {
Expand Down