Improve error message for [build] structure errors (#890)

Author: perazz

src/fpm/manifest/build.f90

@@ -52,20 +52,23 @@ module fpm_manifest_build
 
 
     !> Construct a new build configuration from a TOML data structure
-    subroutine new_build_config(self, table, error)
+    subroutine new_build_config(self, table, package_name, error)
 
         !> Instance of the build configuration
         type(build_config_t), intent(out) :: self
 
         !> Instance of the TOML data structure
         type(toml_table), intent(inout) :: table
 
+        !> Package name
+        character(len=*), intent(in) :: package_name
+
         !> Error handling
         type(error_t), allocatable, intent(out) :: error
 
         integer :: stat
 
-        call check(table, error)
+        call check(table, package_name, error)
         if (allocated(error)) return
 
         call get_value(table, "auto-executables", self%auto_executables, .true., stat=stat)
@@ -128,11 +131,14 @@ subroutine new_build_config(self, table, error)
     end subroutine new_build_config
 
     !> Check local schema for allowed entries
-    subroutine check(table, error)
+    subroutine check(table, package_name, error)
 
         !> Instance of the TOML data structure
         type(toml_table), intent(inout) :: table
 
+        !> Package name
+        character(len=*), intent(in) :: package_name
+
         !> Error handling
         type(error_t), allocatable, intent(out) :: error
 
@@ -154,7 +160,9 @@ subroutine check(table, error)
                 continue
 
             case default
-                call syntax_error(error, "Key "//list(ikey)%key//" is not allowed in [build]")
+
+                call syntax_error(error, 'Manifest file syntax error: key "'//list(ikey)%key//'" found in the [build] '//&
+                                         'section of package/dependency "'//package_name//'" fpm.toml is not allowed')
                 exit
 
             end select

src/fpm/manifest/package.f90

@@ -172,7 +172,7 @@ subroutine new_package(self, table, root, error)
             call fatal_error(error, "Type mismatch for build entry, must be a table")
             return
         end if
-        call new_build_config(self%build, child, error)
+        call new_build_config(self%build, child, self%name, error)
         if (allocated(error)) return
 
         call get_value(table, "install", child, requested=.true., stat=stat)
@@ -232,7 +232,7 @@ subroutine new_package(self, table, root, error)
             call new_library(self%library, child, error)
             if (allocated(error)) return
         end if
-        
+
         call get_value(table, "profiles", child, requested=.false.)
         if (associated(child)) then
             call new_profiles(self%profiles, child, error)
@@ -442,7 +442,7 @@ subroutine info(self, unit, verbosity)
                 call self%dev_dependency(ii)%info(unit, pr - 1)
             end do
         end if
-        
+
         if (allocated(self%profiles)) then
             if (size(self%profiles) > 1 .or. pr > 2) then
                 write(unit, fmti) "- profiles", size(self%profiles)

test/fpm_test/test_manifest.f90

@@ -5,6 +5,7 @@ module test_manifest
     use fpm_manifest
     use fpm_manifest_profile, only: profile_config_t, find_profile
     use fpm_strings, only: operator(.in.)
+    use fpm_error, only: fatal_error, error_t
     implicit none
     private
     public :: collect_manifest
@@ -42,6 +43,7 @@ subroutine collect_manifest(tests)
             & new_unittest("build-config-valid", test_build_valid), &
             & new_unittest("build-config-empty", test_build_empty), &
             & new_unittest("build-config-invalid-values", test_build_invalid_values, should_fail=.true.), &
+            & new_unittest("build-key-invalid", test_build_invalid_key), &
             & new_unittest("library-empty", test_library_empty), &
             & new_unittest("library-wrongkey", test_library_wrongkey, should_fail=.true.), &
             & new_unittest("package-simple", test_package_simple), &
@@ -693,6 +695,52 @@ subroutine test_build_valid(error)
     end subroutine test_build_valid
 
 
+    !> Try to read values from the [build] table
+    subroutine test_build_invalid_key(error)
+
+        !> Error handling
+        type(error_t), allocatable, intent(out) :: error
+
+        type(package_config_t) :: package
+        character(:), allocatable :: temp_file
+        integer :: unit
+        type(error_t), allocatable :: build_error
+
+        allocate(temp_file, source=get_temp_filename())
+
+        open(file=temp_file, newunit=unit)
+        write(unit, '(a)') &
+            & 'name = "example"', &
+            & '[build]', &
+            & 'auto-executables = false', &
+            & 'auto-tests = false ', &
+            & 'module-naming = true ', &
+            & 'this-will-fail = true '
+        close(unit)
+
+        call get_package_data(package, temp_file, build_error)
+
+        ! Error message should contain both package name and key name
+        if (allocated(build_error)) then
+
+            if (.not.index(build_error%message,'this-will-fail')>0) then
+                call fatal_error(error, 'no invalid key name is printed to output')
+                return
+            end if
+
+            if (.not.index(build_error%message,'example')>0) then
+                call fatal_error(error, 'no package name is printed to output')
+                return
+            end if
+
+        else
+            call fatal_error(error, 'no error allocated on invalid [build] section key ')
+            return
+        end if
+
+    end subroutine test_build_invalid_key
+
+
     !> Try to read values from an empty [build] table
     subroutine test_build_empty(error)
 
@@ -1156,7 +1204,7 @@ subroutine test_link_string(error)
         table = toml_table()
         call set_value(table, "link", "z", stat=stat)
 
-        call new_build_config(build, table, error)
+        call new_build_config(build, table, 'test_link_string', error)
 
     end subroutine test_link_string
 
@@ -1179,7 +1227,7 @@ subroutine test_link_array(error)
         call set_value(children, 1, "blas", stat=stat)
         call set_value(children, 2, "lapack", stat=stat)
 
-        call new_build_config(build, table, error)
+        call new_build_config(build, table, 'test_link_array', error)
 
     end subroutine test_link_array
 
@@ -1200,7 +1248,7 @@ subroutine test_invalid_link(error)
         table = toml_table()
         call add_table(table, "link", child, stat=stat)
 
-        call new_build_config(build, table, error)
+        call new_build_config(build, table, 'test_invalid_link', error)
 
     end subroutine test_invalid_link