Coding style checker about codeigniter4

[kenjis]

I want the official coding style checke/fixer for CI4.
If we have it, we can check the style automatically, and we could reject bad style PR automatically. ;-)

[lonnieezell]

One does not exist, currently. I've got the settings generally in my IDE (PHPStorm) but haven't messed with anything else yet. Honestly, don't have experience with those, either.

[lonnieezell]

Yes, it would be nice, and we can probably get to that at some point. :)

[jim-parry]

@kenjis Is the current styleguide workable for you?
If so, it would be great, IMHO, to add the style check to the travis-ci build :)

[lonnieezell]

I found this site this morning that looks like a great way to integrate: https://styleci.readme.io/

I've signed up for the repo but haven't integrated, yet. Will try to do that in the next day or so.

[mwhitneysdsu]

I made some minor changes to the styleguide to correct some errors. Most of the better coding style checkers/fixers allow the user to configure the enforced rules, so we can probably create (or accept contributions of) the necessary files to support the coding style defined in the guide.

[lonnieezell]

yes, it supports that - and that is what I haven't configured yet. The plan is to do that tonight if I can break free from work.

[mwhitneysdsu]

I was just thinking in general, as you brought up StyleCI, kenjis brought up php-cs-fixer, and I've done a little work on configuring PHP_CodeSniffer (which I use in SublimeText). I'm not really the kind of person that wants to wait until CI checks run on the repo before I find out that the code doesn't match the style guide, because I know quite well that I'm not going to use Allman-style braces without a tool to remind me (or force me) to do so.

[lonnieezell]

Completely understand. StyleCI seems to run codesniffer (I think, looking at the "fixer" names), but provides tools to generate pull requests to fix common items, etc. Also, it integrates right into GitHub, much like Travis does, so we can see if there are errors before merging. Seems ideal for working with lots of contributors.

I've got PHPStorm setup to handle the styling for me, and had started to setup codesniffer config locally, but never got it finished. That would be nice to have, though.

[lonnieezell]

I got us setup on StyleCI, but it's throwing an error on our Config/Hooks file. Probably because it's mostly a placeholder file. I've got an email into their support, so we'll where that takes us. It doesn't support all of the different things our styleguide does, but it will help keep many things standardized.

[jim-parry]

Looking forward to where this takes us!

[jim-parry]

The updated styleci config still fails, and now when I click the badge, and then "show details", it shows the sorce code changes without an indication of why something might have failed the style check?

[lonnieezell]

Correct. I tweaked the styles last night to remove a good portion of the
extra whitespace checks. It will show fail until we resolve the current
errors, which is a matter of confirming that the styles being applied are
what we want, and then having it submit a PR to update them. I only got to
look through about 20% of the file last night, but it was looking pretty
good.

[lonnieezell]

While StyleCI does some cool things for us, there are a few things it does that don't work and I haven't found the checkers that control that, so I think I'll remove it for now. We can re-evaluate later, or look at different tools.

[jim-parry]

Agreed.

[timw4mail]

Just a thought: Editorconfig is a popular way to enforce basic coding style at an editor level. It just involves a .editorconfig file in the root, and would act as a passive starting point for code style.

Here's an example of a fairly general .editorconfig file:

# EditorConfig is awesome: http://EditorConfig.org

# top-most EditorConfig file
root = true

# Unix-style newlines with a newline ending every file
[*]
end_of_line = lf
insert_final_newline = false
charset = utf-8
indent_style = tab
trim_trailing_whitespace = true

[*.{cpp,c,h,hpp,cxx}]
insert_final_newline = true

# Yaml files
[*.{yml,yaml}]
indent_style = space
indent_size = 4

[mwhitneysdsu]

While Editorconfig looks promising, most of the settings it supports are included in your example, which isn't going to help with most of the things that come up with coding style issues. One of the glaring issues for this project is that it does not support configuring the use of tabs for indentation while using spaces for alignment.

[timw4mail]

@mwhitneysdsu I'm aware it isn't very powerful, but it at least has the potential to help get rid of space indentation.

[mwhitneysdsu]

@timw4mail It may get rid of space indentation, but tabs used in alignment are a significant problem as well, and more difficult to correct if you don't have a script that can correct it for you.

Personally, I only use CI4's coding style when working on CI4 itself, so I've spent a lot of time trying to configure my editor and linter to behave properly in each of my projects, so it would be nice if this were as simple as downloading a file with the project. However, I'm still going to need to install/configure a linter and some other plug-ins to really check/enforce the style guide.

I think Editorconfig is a good idea for some users, but there are better ways of dealing with this problem, in my opinion. Ideally, if we could get a good configuration for PHP CS Fixer, it wouldn't matter what the individual users' editor settings were, because PHP CS Fixer would be able to do its job and reformat the file to meet the requirements.

Until then, I just use the following project file in SublimeText (actually, this is simplified a bit), along with some rules-in-progress for PHP CodeSniffer (which is run by SublimeText on save) to tell me when things aren't right.

{
    "folders":
    [
        {
            "path": "/your/path/here"
        }
    ],
    "settings":
    {
        "default_encoding": "UTF-8",
        "default_line_ending": "unix",
        "translate_tabs_to_spaces": false,
        "tab_size": 4,
        "trim_trailing_white_space_on_save": true,
        "ensure_newline_at_eof_on_save": true
    }
}

[groovenectar]

I did notice (in system/Commands) that some files are using 4 indentation characters for a single indent level. That had me concerned until I saw the rest of the project continues to use tabs for tabulation.

Probably sounds weird to say, but it would be heartbreaking if the Codeignitor project in general started using (x number of) space characters to represent a tab character.

PSR. is. wrong. (about that specific issue). And it has influenced many projects in the wrong direction. These are typically web projects, and if we get used to spaces for indentation it could bleed into our HTML/JS and in some cases add +/- 20% to filesize.

Using an .editorconfig file would be a great idea and is very well-supported.

[groovenectar]

Using EditorConfig with PHPStorm, SublimeText and Atom at least, I've not personally run into any issues with spaces for character alignment and tabs for tabulation/indentation. If an editor botches that, then it would have done that anyway, and I feel like that would be really rare. I've not run into that upon opening a file, and EditorConfig isn't designed to change anything upon open/save, just for the editing process. The code style problem is already solved before the file is opened for editing, and before new files are created... I would be curious to know others' findings on that...

[jim-parry]

@lonnieezell Is this superceded by #182 ? It looks like this issue can be closed.

[lonnieezell]

@jim-parry we should be good with just the other one. Closing.