search

MaciejPatro / cmake-tidy

Project provides a set of tools that will ease the development process for code written in CMake.
MIT License
15 stars 0 forks source link

Keyword arguments may need further categorization (a.k.a. misleading options formatting) #146

Open mikedld opened 4 years ago

mikedld commented 4 years ago

Just as cmake_parse_arguments distincts between "options", "one-value keywords", and "multi-value keywords", cmake-tidy may need to as well. Currently, formatting may yield odd results for e.g. add_executable or add_library.

Input (source files list shortened for the purposes of this issue):

add_executable(foo WIN32
    bar
    baz)

add_library(foo STATIC
    bar
    baz)

Expected output: same as input (?) Actual output:

add_executable(foo
    WIN32
        bar
        baz)

add_library(foo
    STATIC
        bar
        baz)

Although the results looks kind of okay, they are inconsistent with other formatting in that they give an impression that "bar" and "baz" are values of WIN32 and STATIC keywords which is not true.

With decent CMake version one may want to workaround this by using target_sources which would be formatted properly, as in

add_executable(foo WIN32)

target_sources(foo
    PRIVATE
        bar
        baz)
MaciejPatro commented 4 years ago

Hi,

I agree that this is an issue and it needs some action. I thought some time about it and first I would need a bit more investigation about the problem before choosing a solution (eg. cmake_parse_arguments approach).

Some points that need to be taken into account: (Sorry that the points are quite unsystematic - there are mainly notes made during the day)

  1. Keywords change meaning for different commands. Example:
    • INTERFACE as option: 
      add_library(a_library INTERFACE)
    • INTERFACE as multi-value keyword: 
      target_sources(a_target 
INTERFACE
    a_header.hpp
    another_one.hpp
)

      This means that cmake-tidy would need to have information about keyword type connected to each command. Which leads to couple of next questions:

    • Maintaining the information. #144 gave quite a nice idea about generating list of properties, similarly acquiring command names should be easy - but getting all keywords with type (option/argument keyword) will be more complex problem.
    • User defined functions/macros formatting - how to handle it? Manual definition of settings in configuration file (with type distinction) can be quite complex.
      • Reasonable defaults? Inheriting from existing builtin command?
      • There could be a convenience script that would generate it based on cmake_parse_arguments.
      • Remark: It is (at least for now) not a goal for the tool to resolve includes and find proper definitions of user-defined functions/macros
    • CMake changes in builtin commands (adding/deprecating keywords) - how will they affect formatting with older/newer version of cmake-tidy
  2. cmake_parse_arguments handling is not enough to properly format all cases (for builtin commands): example https://cmake.org/cmake/help/latest/command/install.html#command:install 
    install(TARGETS mylib
   LIBRARY
       COMPONENT Libraries
       NAMELINK_COMPONENT Development
       PERMISSIONS GROUP_READ OWNER_WRITE
   PUBLIC_HEADER
       COMPONENT Development
)
  3. Change of handling cmake_parse_arguments command handling by CMake will most likely result in breaking change for users of cmake-tidy at least for configurations.

I will play around with the idea on a branch and come back with some more thoughts/answers.