Better error reporting; better nullptr safety.

Also clears the marker lists before running a DRC, and sets
the drcRun flags afterwards so that the notebook tab counts
get updated.

Fixes https://gitlab.com/kicad/code/kicad/issues/5659

pcbnew/dialogs/dialog_drc.cpp

@@ -115,7 +115,7 @@ void DIALOG_DRC::OnActivateDlg( wxActivateEvent& aEvent )
         Close();
 
         DRC_TOOL* drcTool = m_brdEditor->GetToolManager()->GetTool<DRC_TOOL>();
-        drcTool->DestroyDRCDialog( wxID_CANCEL );
+        drcTool->DestroyDRCDialog();
 
         return;
     }
@@ -185,8 +185,6 @@ void DIALOG_DRC::SetCurrentProgress( double aProgress )
 }
 
 
-
-
 // Don't globally define this; different facilities use different definitions of "ALL"
 static int RPT_SEVERITY_ALL = RPT_SEVERITY_WARNING | RPT_SEVERITY_ERROR | RPT_SEVERITY_EXCLUSION;
 
@@ -213,9 +211,9 @@ void DIALOG_DRC::OnRunDRCClick( wxCommandEvent& aEvent )
 
     m_brdEditor->RecordDRCExclusions();
     deleteAllMarkers( true );
+    m_unconnectedTreeModel->DeleteItems( false, true, true );
+    m_footprintWarningsTreeModel->DeleteItems( false, true, true );
 
-    wxBeginBusyCursor();
-    wxWindowDisabler disabler( this );
     Raise();
 
     m_Notebook->ChangeSelection( 0 ); // Display the "Messages" tab
@@ -224,12 +222,6 @@ void DIALOG_DRC::OnRunDRCClick( wxCommandEvent& aEvent )
 
     drcTool->RunTests( this, testTracksAgainstZones, refillZones, reportAllTrackErrors,
                        testFootprints );
-    m_drcRun = true;
-
-    if( testFootprints )
-        m_footprintTestsRun = true;
-
-    wxEndBusyCursor();
 
     refreshBoardEditor();
 
@@ -569,7 +561,7 @@ void DIALOG_DRC::OnCancelClick( wxCommandEvent& aEvent )
     // The dialog can be modal or not modal.
     // Leave the DRC caller destroy (or not) the dialog
     DRC_TOOL* drcTool = m_brdEditor->GetToolManager()->GetTool<DRC_TOOL>();
-    drcTool->DestroyDRCDialog( wxID_CANCEL );
+    drcTool->DestroyDRCDialog();
 }
 
 

pcbnew/dialogs/dialog_drc.h

@@ -50,6 +50,9 @@ public:
     DIALOG_DRC( PCB_EDIT_FRAME* aEditorFrame, wxWindow* aParent );
     ~DIALOG_DRC();
 
+    void SetDrcRun() { m_drcRun = true; }
+    void SetFootprintTestsRun() { m_footprintTestsRun = true; }
+
     void SetMarkersProvider( RC_ITEMS_PROVIDER* aProvider );
     void SetUnconnectedProvider( RC_ITEMS_PROVIDER* aProvider );
     void SetFootprintsProvider( RC_ITEMS_PROVIDER* aProvider );

pcbnew/dialogs/dialog_drc_base.cpp

@@ -72,7 +72,7 @@ DIALOG_DRC_BASE::DIALOG_DRC_BASE( wxWindow* parent, wxWindowID id, const wxStrin
 	m_panelMessages->SetSizer( bSizer10 );
 	m_panelMessages->Layout();
 	bSizer10->Fit( m_panelMessages );
-	m_Notebook->AddPage( m_panelMessages, _("Messages"), false );
+	m_Notebook->AddPage( m_panelMessages, _("Messages"), true );
 	m_panelViolations = new wxPanel( m_Notebook, wxID_ANY, wxDefaultPosition, wxDefaultSize, wxTAB_TRAVERSAL );
 	wxBoxSizer* bSizerViolationsBox;
 	bSizerViolationsBox = new wxBoxSizer( wxVERTICAL );
@@ -87,7 +87,7 @@ DIALOG_DRC_BASE::DIALOG_DRC_BASE( wxWindow* parent, wxWindowID id, const wxStrin
 	m_panelViolations->SetSizer( bSizerViolationsBox );
 	m_panelViolations->Layout();
 	bSizerViolationsBox->Fit( m_panelViolations );
-	m_Notebook->AddPage( m_panelViolations, _("Violations (%d)"), true );
+	m_Notebook->AddPage( m_panelViolations, _("Violations (%d)"), false );
 	m_panelUnconnectedItems = new wxPanel( m_Notebook, wxID_ANY, wxDefaultPosition, wxDefaultSize, wxTAB_TRAVERSAL );
 	wxBoxSizer* bSizerUnconnectedBox;
 	bSizerUnconnectedBox = new wxBoxSizer( wxVERTICAL );

pcbnew/dialogs/dialog_drc_base.fbp

@@ -409,7 +409,7 @@
                         <object class="notebookpage" expanded="1">
                             <property name="bitmap"></property>
                             <property name="label">Messages</property>
-                            <property name="select">0</property>
+                            <property name="select">1</property>
                             <object class="wxPanel" expanded="1">
                                 <property name="BottomDockable">1</property>
                                 <property name="LeftDockable">1</property>
@@ -611,7 +611,7 @@
                         <object class="notebookpage" expanded="1">
                             <property name="bitmap"></property>
                             <property name="label">Violations (%d)</property>
-                            <property name="select">1</property>
+                            <property name="select">0</property>
                             <object class="wxPanel" expanded="1">
                                 <property name="BottomDockable">1</property>
                                 <property name="LeftDockable">1</property>

pcbnew/dialogs/dialog_plot.cpp

@@ -907,7 +907,7 @@ void DIALOG_PLOT::onRunDRC( wxCommandEvent& event )
 
         // First close an existing dialog if open
         // (low probability, but can happen)
-        drcTool->DestroyDRCDialog( wxID_OK );
+        drcTool->DestroyDRCDialog();
 
         // Open a new drc dialod, with the right parent frame, and in Modal Mode
         drcTool->ShowDRCDialog( this );

pcbnew/dialogs/panel_setup_rules.cpp

@@ -294,7 +294,7 @@ void PANEL_SETUP_RULES::OnCompile( wxCommandEvent& event )
     {
         std::vector<DRC_RULE*> dummyRules;
 
-        DRC_RULES_PARSER parser( m_frame->GetBoard(), m_textEditor->GetText(), _( "DRC rules" ) );
+        DRC_RULES_PARSER parser( m_textEditor->GetText(), _( "DRC rules" ) );
 
         parser.Parse( dummyRules, m_errorsReport );
     }
@@ -363,7 +363,7 @@ bool PANEL_SETUP_RULES::TransferDataFromWindow()
     {
         std::vector<DRC_RULE*> dummyRules;
 
-        DRC_RULES_PARSER parser( m_frame->GetBoard(), m_textEditor->GetText(), _( "DRC rules" ) );
+        DRC_RULES_PARSER parser( m_textEditor->GetText(), _( "DRC rules" ) );
 
         parser.Parse( dummyRules, m_errorsReport );
     }
@@ -375,9 +375,18 @@ bool PANEL_SETUP_RULES::TransferDataFromWindow()
 
     wxString rulesFilepath = m_frame->Prj().AbsolutePath( "drc-rules" );
 
-    if( m_textEditor->SaveFile( rulesFilepath ) )
+    try
     {
-        m_frame->GetBoard()->GetDesignSettings().m_DRCEngine->LoadRules( rulesFilepath );
+        if( m_textEditor->SaveFile( rulesFilepath ) )
+        {
+            m_frame->GetBoard()->GetDesignSettings().m_DRCEngine->LoadRules( rulesFilepath );
+            return true;
+        }
+    }
+    catch( PARSE_ERROR& pe )
+    {
+        // Don't lock them in to the Setup dialog if they have bad rules.  They've already
+        // saved them so we can allow an exit.
         return true;
     }
 

pcbnew/drc/drc_engine.cpp

@@ -260,11 +260,11 @@ static wxString formatConstraint( const DRC_CONSTRAINT& constraint )
 }
 
 
-bool DRC_ENGINE::LoadRules( const wxFileName& aPath )
+/**
+ * @throws PARSE_ERROR
+ */
+void DRC_ENGINE::LoadRules( const wxFileName& aPath )
 {
-    NULL_REPORTER nullReporter;
-    REPORTER*     reporter = m_reporter ? m_reporter : &nullReporter;
-
     if( aPath.FileExists() )
     {
         m_ruleConditions.clear();
@@ -276,8 +276,8 @@ bool DRC_ENGINE::LoadRules( const wxFileName& aPath )
         {
             try
             {
-                DRC_RULES_PARSER parser( m_board, fp, aPath.GetFullPath() );
+                DRC_RULES_PARSER parser( fp, aPath.GetFullPath() );
-                parser.Parse( m_rules, reporter );
+                parser.Parse( m_rules, m_reporter );
             }
             catch( PARSE_ERROR& pe )
             {
@@ -285,17 +285,10 @@ bool DRC_ENGINE::LoadRules( const wxFileName& aPath )
                 m_ruleConditions.clear();
                 m_rules.clear();
 
-                // JEY TODO
+                throw pe;
-                //wxSafeYield( m_editFrame );
-                //m_editFrame->ShowBoardSetupDialog( _( "Rules" ), pe.What(), ID_RULES_EDITOR,
-                //                                   pe.lineNumber, pe.byteIndex );
-
-                return false;
             }
         }
     }
-
-    return true;
 }
 
 
@@ -375,6 +368,9 @@ bool DRC_ENGINE::CompileRules()
 }
 
 
+/**
+ * @throws PARSE_ERROR
+ */
 void DRC_ENGINE::InitEngine( const wxFileName& aRulePath )
 {
     m_testProviders = DRC_TEST_PROVIDER_REGISTRY::Instance().GetTestProviders();
@@ -442,6 +438,8 @@ DRC_CONSTRAINT DRC_ENGINE::EvalRulesForItems( DRC_CONSTRAINT_TYPE_T aConstraintI
 
     const BOARD_CONNECTED_ITEM* connectedA = dynamic_cast<const BOARD_CONNECTED_ITEM*>( a );
     const BOARD_CONNECTED_ITEM* connectedB = dynamic_cast<const BOARD_CONNECTED_ITEM*>( b );
+    const DRC_CONSTRAINT*       constraintRef = nullptr;
+    bool                        implicit = false;
 
     // Local overrides take precedence
     if( aConstraintId == DRC_CONSTRAINT_TYPE_CLEARANCE )
@@ -477,80 +475,83 @@ DRC_CONSTRAINT DRC_ENGINE::EvalRulesForItems( DRC_CONSTRAINT_TYPE_T aConstraintI
         }
     }
 
-    std::vector<CONSTRAINT_WITH_CONDITIONS*>* ruleset = m_constraintMap[ aConstraintId ];
+    if( m_constraintMap.count( aConstraintId ) )
-
-    const DRC_CONSTRAINT* constraintRef = nullptr;
-    bool                  implicit = false;
-
-    // Last matching rule wins, so process in reverse order
-    for( int ii = (int) ruleset->size() - 1; ii >= 0; --ii )
     {
-        wxString msg;
+        std::vector<CONSTRAINT_WITH_CONDITIONS*>* ruleset = m_constraintMap[ aConstraintId ];
-        const CONSTRAINT_WITH_CONDITIONS* rcons = ruleset->at( ii );
-        implicit = rcons->parentRule && rcons->parentRule->m_Implicit;
 
-        REPORT( "" )
+        // Last matching rule wins, so process in reverse order
-
+        for( int ii = (int) ruleset->size() - 1; ii >= 0; --ii )
-        if( implicit )
         {
-            msg = wxString::Format( _( "Checking %s;" ),
+            wxString msg;
-                                    rcons->parentRule->m_Name );
+            const CONSTRAINT_WITH_CONDITIONS* rcons = ruleset->at( ii );
-        }
+            implicit = rcons->parentRule && rcons->parentRule->m_Implicit;
-        else
-        {
-            msg = wxString::Format( _( "Checking rule %s;"),
-                                    rcons->parentRule->m_Name );
-        }
 
-        if( aConstraintId == DRC_CONSTRAINT_TYPE_CLEARANCE )
+            REPORT( "" )
-        {
-            int clearance = rcons->constraint.m_Value.Min();
 
-            msg += " ";
+            if( implicit )
-            msg += wxString::Format( _( "clearance: %s." ),
-                                     MessageTextFromValue( UNITS, clearance, true ) );
-        }
-
-        REPORT( msg );
-
-        if( aLayer != UNDEFINED_LAYER && !rcons->layerTest.test( aLayer ) )
-        {
-            REPORT( wxString::Format( _( "Rule layer \"%s\" not matched." ),
-                                      rcons->parentRule->m_LayerSource ) )
-            REPORT( "Rule not applied." )
-
-            continue;
-        }
-
-        if( !rcons->condition || rcons->condition->GetExpression().IsEmpty() )
-        {
-            REPORT( implicit ? _( "Unconditional constraint applied." )
-                             : _( "Unconditional rule applied." ) )
-
-            constraintRef = &rcons->constraint;
-            break;
-        }
-        else
-        {
-            // Don't report on implicit rule conditions; they're synthetic.
-            if( !implicit )
             {
-                REPORT( wxString::Format( _( "Checking rule condition \"%s\"." ),
+                msg = wxString::Format( _( "Checking %s;" ),
-                                          rcons->condition->GetExpression() ) )
+                                        rcons->constraint.GetName() );
+            }
+            else
+            {
+                msg = wxString::Format( _( "Checking rule %s;"),
+                                        rcons->constraint.GetName() );
             }
 
-            if( rcons->condition->EvaluateFor( a, b, aLayer, aReporter ) )
+            if( aConstraintId == DRC_CONSTRAINT_TYPE_CLEARANCE )
             {
-                REPORT( implicit ? _( "Constraint applicable." )
+                int clearance = rcons->constraint.m_Value.Min();
-                                 : _( "Rule applied." ) )
+
+                msg += " ";
+                msg += wxString::Format( _( "clearance: %s." ),
+                                         MessageTextFromValue( UNITS, clearance, true ) );
+            }
+
+            REPORT( msg );
+
+            if( aLayer != UNDEFINED_LAYER && !rcons->layerTest.test( aLayer ) )
+            {
+                if( rcons->parentRule )
+                {
+                    REPORT( wxString::Format( _( "Rule layer \"%s\" not matched." ),
+                                              rcons->parentRule->m_LayerSource ) )
+                    REPORT( "Rule not applied." )
+                }
+
+                continue;
+            }
+
+            if( !rcons->condition || rcons->condition->GetExpression().IsEmpty() )
+            {
+                REPORT( implicit ? _( "Unconditional constraint applied." )
+                                 : _( "Unconditional rule applied." ) )
 
                 constraintRef = &rcons->constraint;
                 break;
             }
             else
             {
-                REPORT( implicit ? _( "Membership not satisfied; constraint not applicable." )
+                // Don't report on implicit rule conditions; they're synthetic.
-                                 : _( "Condition not satisfied; rule not applied." ) )
+                if( !implicit )
+                {
+                    REPORT( wxString::Format( _( "Checking rule condition \"%s\"." ),
+                                              rcons->condition->GetExpression() ) )
+                }
+
+                if( rcons->condition->EvaluateFor( a, b, aLayer, aReporter ) )
+                {
+                    REPORT( implicit ? _( "Constraint applicable." )
+                                     : _( "Rule applied." ) )
+
+                    constraintRef = &rcons->constraint;
+                    break;
+                }
+                else
+                {
+                    REPORT( implicit ? _( "Membership not satisfied; constraint not applicable." )
+                                     : _( "Condition not satisfied; rule not applied." ) )
+                }
             }
         }
     }
@@ -695,8 +696,11 @@ std::vector<DRC_CONSTRAINT> DRC_ENGINE::QueryConstraintsById( DRC_CONSTRAINT_TYP
 {
     std::vector<DRC_CONSTRAINT> rv;
 
-    for ( CONSTRAINT_WITH_CONDITIONS* c : *m_constraintMap[constraintID] )
+    if( m_constraintMap.count( constraintID ) )
-        rv.push_back( c->constraint );
+    {
+        for ( CONSTRAINT_WITH_CONDITIONS* c : *m_constraintMap[ constraintID ] )
+            rv.push_back( c->constraint );
+    }
 
     return rv;
 }
@@ -705,7 +709,10 @@ std::vector<DRC_CONSTRAINT> DRC_ENGINE::QueryConstraintsById( DRC_CONSTRAINT_TYP
 bool DRC_ENGINE::HasRulesForConstraintType( DRC_CONSTRAINT_TYPE_T constraintID )
 {
     //drc_dbg(10,"hascorrect id %d size %d\n", ruleID,  m_ruleMap[ruleID]->sortedRules.size( ) );
-    return m_constraintMap[constraintID]->size() != 0;
+    if( m_constraintMap.count( constraintID ) )
+        return m_constraintMap[ constraintID ]->size() > 0;
+
+    return false;
 }
 
 

pcbnew/drc/drc_engine.h

@@ -69,12 +69,14 @@ std::function<void( const std::shared_ptr<DRC_ITEM>& aItem, wxPoint aPos )> DRC_
 
 
 /**
- * Design Rule Checker object that performs all the DRC tests.  The output of
+ * Design Rule Checker object that performs all the DRC tests.
- * the checking goes to the BOARD file in the form of two MARKER lists.  Those
+ *
- * two lists are displayable in the drc dialog box.  And they can optionally
+ * Optionally reports violations via a DRC_VIOLATION_HANDLER, user-level progress via a
- * be sent to a text file on disk.
+ * PROGRESS_REPORTER and rule parse errors via a REPORTER, all set through various setter
- * This class is given access to the windows and the BOARD
+ * calls.
- * that it needs via its constructor or public access functions.
+ *
+ * Note that EvalRulesForItems() has yet another optional REPORTER for reporting resolution
+ * info to the user.
  */
 class DRC_ENGINE
 {
@@ -88,8 +90,8 @@ public:
     void SetWorksheet( KIGFX::WS_PROXY_VIEW_ITEM* aWorksheet ) { m_worksheet = aWorksheet; }
     KIGFX::WS_PROXY_VIEW_ITEM* GetWorksheet() const { return m_worksheet; }
 
-    /*
+    /**
-     * Receives DRC violation reports (a DRC_ITEM and a position).
+     * Set an optional DRC violation handler (receives DRC_ITEMs and positions).
      */
     void SetViolationHandler( DRC_VIOLATION_HANDLER aHandler )
     {
@@ -101,23 +103,41 @@ public:
         m_violationHandler = DRC_VIOLATION_HANDLER();
     }
 
-    /*
+    /**
-     * Receives progress information to show the user.
+     * Set an optional reporter for user-level progress info.
      */
     void SetProgressReporter( PROGRESS_REPORTER* aProgRep ) { m_progressReporter = aProgRep; }
 
     /*
-     * Receives debug information for a developer.
+     * Set an optional reporter for rule parse/compile/run-time errors and log-level progress
+     * information.
+     *
+     * Note: if no log reporter is installed rule parse/compile/run-time errors are returned
+     * via a thrown PARSE_ERROR exception.
      */
     void SetLogReporter( REPORTER* aReporter ) { m_reporter = aReporter; }
 
-    /*
+    /**
      * Loads and parses a rule set from an sexpr text file.
+     *
+     * @throws PARSE_ERROR
      */
-    bool LoadRules( const wxFileName& aPath );
+    void LoadRules( const wxFileName& aPath );
 
+    /**
+     * Initializes the DRC engine.
+     *
+     * @throws PARSE_ERROR if the rules file contains errors
+     */
     void InitEngine( const wxFileName& aRulePath );
 
+    /**
+     * Runs the DRC tests.
+     * @param aUnits
+     * @param aTestTracksAgainstZones
+     * @param aReportAllTrackErrors
+     * @param aTestFootprints
+     */
     void RunTests( EDA_UNITS aUnits = EDA_UNITS::MILLIMETRES, bool aTestTracksAgainstZones = true,
                    bool aReportAllTrackErrors = true, bool aTestFootprints = true );
 

pcbnew/drc/drc_rule_parser.cpp

@@ -33,8 +33,7 @@
 using namespace DRCRULE_T;
 
 
-DRC_RULES_PARSER::DRC_RULES_PARSER( BOARD* aBoard, const wxString& aSource,
+DRC_RULES_PARSER::DRC_RULES_PARSER( const wxString& aSource, const wxString& aSourceDescr ) :
-                                    const wxString& aSourceDescr ) :
         DRC_RULES_LEXER( aSource.ToStdString(), aSourceDescr ),
         m_requiredVersion( 0 ),
         m_tooRecent( false ),
@@ -43,7 +42,7 @@ DRC_RULES_PARSER::DRC_RULES_PARSER( BOARD* aBoard, const wxString& aSource,
 }
 
 
-DRC_RULES_PARSER::DRC_RULES_PARSER( BOARD* aBoard, FILE* aFile, const wxString& aFilename ) :
+DRC_RULES_PARSER::DRC_RULES_PARSER( FILE* aFile, const wxString& aFilename ) :
         DRC_RULES_LEXER( aFile, aFilename ),
         m_requiredVersion( 0 ),
         m_tooRecent( false ),
@@ -56,13 +55,25 @@ void DRC_RULES_PARSER::reportError( const wxString& aMessage )
 {
     wxString rest;
     wxString first = aMessage.BeforeFirst( '|', &rest );
-    wxString msg = wxString::Format( _( "ERROR: <a href='%d:%d'>%s</a>%s" ),
-                                     CurLineNumber(),
-                                     CurOffset(),
-                                     first,
-                                     rest );
 
-    m_reporter->Report( msg, RPT_SEVERITY_ERROR );
+    if( m_reporter )
+    {
+        wxString msg = wxString::Format( _( "ERROR: <a href='%d:%d'>%s</a>%s" ),
+                                         CurLineNumber(),
+                                         CurOffset(),
+                                         first,
+                                         rest );
+
+        m_reporter->Report( msg, RPT_SEVERITY_ERROR );
+    }
+    else
+    {
+        wxString msg = wxString::Format( _( "ERROR: %s%s" ),
+                                         first,
+                                         rest );
+
+        THROW_PARSE_ERROR( msg, CurSource(), CurLine(), CurLineNumber(), CurOffset() );
+    }
 }
 
 
@@ -124,13 +135,15 @@ void DRC_RULES_PARSER::Parse( std::vector<DRC_RULE*>& aRules, REPORTER* aReporte
             }
             else
             {
-                msg.Printf( _( "Unrecognized item '%s'.| Expected version number" ), FromUTF8() );
+                msg.Printf( _( "Unrecognized item '%s'.| Expected version number" ),
+                            FromUTF8() );
                 reportError( msg );
             }
 
             if( (int) token != DSN_RIGHT )
             {
-                msg.Printf( _( "Unrecognized item '%s'." ), FromUTF8() );
+                msg.Printf( _( "Unrecognized item '%s'." ),
+                            FromUTF8() );
                 reportError( msg );
                 parseUnknown();
             }
@@ -147,13 +160,14 @@ void DRC_RULES_PARSER::Parse( std::vector<DRC_RULE*>& aRules, REPORTER* aReporte
 
         default:
             msg.Printf( _( "Unrecognized item '%s'.| Expected %s." ),
-                        FromUTF8(), "'rule', 'version'" );
+                        FromUTF8(),
+                        "'rule', 'version'" );
             reportError( msg );
             parseUnknown();
         }
     }
 
-    if( !m_reporter->HasMessage() )
+    if( m_reporter && !m_reporter->HasMessage() )
         m_reporter->Report( _( "No errors found." ), RPT_SEVERITY_INFO );
 
     m_reporter = nullptr;
@@ -207,7 +221,8 @@ DRC_RULE* DRC_RULES_PARSER::parseDRC_RULE()
 
             if( (int) NextTok() != DSN_RIGHT )
             {
-                reportError( wxString::Format( _( "Unrecognized item '%s'." ), FromUTF8() ) );
+                reportError( wxString::Format( _( "Unrecognized item '%s'." ),
+                                               FromUTF8() ) );
                 parseUnknown();
             }
 
@@ -302,8 +317,8 @@ void DRC_RULES_PARSER::parseConstraint( DRC_RULE* aRule )
             default:
                 msg.Printf( _( "Unrecognized item '%s'.| Expected %s." ),
                             FromUTF8(),
-                            "'track', 'via', 'micro_via', "
+                            "'track', 'via', 'micro_via', 'blind_via', 'pad', 'zone', 'text', "
-                            "'blind_via', 'pad', 'zone', 'text', 'graphic', 'hole'."
+                            "'graphic', 'hole'."
                             );
                 reportError( msg );
                 break;
@@ -339,7 +354,8 @@ void DRC_RULES_PARSER::parseConstraint( DRC_RULE* aRule )
 
             if( (int) NextTok() != DSN_RIGHT )
             {
-                reportError( wxString::Format( _( "Unrecognized item '%s'." ), FromUTF8() ) );
+                reportError( wxString::Format( _( "Unrecognized item '%s'." ),
+                                               FromUTF8() ) );
                 parseUnknown();
             }
 
@@ -359,7 +375,8 @@ void DRC_RULES_PARSER::parseConstraint( DRC_RULE* aRule )
 
             if( (int) NextTok() != DSN_RIGHT )
             {
-                reportError( wxString::Format( _( "Unrecognized item '%s'." ), FromUTF8() ) );
+                reportError( wxString::Format( _( "Unrecognized item '%s'." ),
+                                               FromUTF8() ) );
                 parseUnknown();
             }
 
@@ -379,7 +396,8 @@ void DRC_RULES_PARSER::parseConstraint( DRC_RULE* aRule )
 
             if( (int) NextTok() != DSN_RIGHT )
             {
-                reportError( wxString::Format( _( "Unrecognized item '%s'." ), FromUTF8() ) );
+                reportError( wxString::Format( _( "Unrecognized item '%s'." ),
+                                               FromUTF8() ) );
                 parseUnknown();
             }
 
@@ -391,7 +409,8 @@ void DRC_RULES_PARSER::parseConstraint( DRC_RULE* aRule )
 
         default:
             msg.Printf( _( "Unrecognized item '%s'.| Expected %s." ),
-                        FromUTF8(), "'min', 'max', 'opt'" );
+                        FromUTF8(),
+                        "'min', 'max', 'opt'" );
             reportError( msg );
             parseUnknown();
         }
@@ -410,13 +429,25 @@ void DRC_RULES_PARSER::parseValueWithUnits( const wxString& aExpr, int& aResult
     {
         wxString rest;
         wxString first = aMessage.BeforeFirst( '|', &rest );
-        wxString msg = wxString::Format( _( "ERROR: <a href='%d:%d'>%s</a>%s" ),
-                                         CurLineNumber(),
-                                         CurOffset() + aOffset,
-                                         first,
-                                         rest );
 
-        m_reporter->Report( msg, RPT_SEVERITY_ERROR );
+        if( m_reporter )
+        {
+            wxString msg = wxString::Format( _( "ERROR: <a href='%d:%d'>%s</a>%s" ),
+                                             CurLineNumber(),
+                                             CurOffset(),
+                                             first,
+                                             rest );
+
+            m_reporter->Report( msg, RPT_SEVERITY_ERROR );
+        }
+        else
+        {
+            wxString msg = wxString::Format( _( "ERROR: %s%s" ),
+                                             first,
+                                             rest );
+
+            THROW_PARSE_ERROR( msg, CurSource(), CurLine(), CurLineNumber(), CurOffset() );
+        }
     };
 
     PCB_EXPR_EVALUATOR evaluator;
@@ -459,12 +490,16 @@ LSET DRC_RULES_PARSER::parseLayer()
         }
 
         if( !retVal.any() )
-            reportError( wxString::Format( _( "Unrecognized layer '%s'." ), layerName ) );
+        {
+            reportError( wxString::Format( _( "Unrecognized layer '%s'." ),
+                                           layerName ) );
+        }
     }
 
     if( (int) NextTok() != DSN_RIGHT )
     {
-        reportError( wxString::Format( _( "Unrecognized item '%s'." ), FromUTF8() ) );
+        reportError( wxString::Format( _( "Unrecognized item '%s'." ),
+                                       FromUTF8() ) );
         parseUnknown();
     }
 

pcbnew/drc/drc_rule_parser.h

@@ -40,8 +40,8 @@ class BOARD_ITEM;
 class DRC_RULES_PARSER : public DRC_RULES_LEXER
 {
 public:
-    DRC_RULES_PARSER( BOARD* aBoard, const wxString& aSource, const wxString& aSourceDescr );
+    DRC_RULES_PARSER( const wxString& aSource, const wxString& aSourceDescr );
-    DRC_RULES_PARSER( BOARD* aBoard, FILE* aFile, const wxString& aFilename );
+    DRC_RULES_PARSER( FILE* aFile, const wxString& aFilename );
 
     void Parse( std::vector<DRC_RULE*>& aRules, REPORTER* aReporter );
 

pcbnew/pcb_base_edit_frame.cpp

@@ -133,7 +133,16 @@ void PCB_BASE_EDIT_FRAME::SetBoard( BOARD* aBoard )
     {
         BOARD_DESIGN_SETTINGS& bds = aBoard->GetDesignSettings();
         bds.m_DRCEngine = std::make_shared<DRC_ENGINE>( aBoard, &bds );
-        bds.m_DRCEngine->InitEngine( Prj().AbsolutePath( "drc-rules" ) );
+
+        try
+        {
+            bds.m_DRCEngine->InitEngine( Prj().AbsolutePath( "drc-rules" ) );
+        }
+        catch( PARSE_ERROR& pe )
+        {
+            // TODO: We could redirect to Board Setup here and report the error.  Or we could
+            // wait till they run DRC or do an Inspect Clearance.  Not sure which is better....
+        }
 
         if( m_toolManager )
             m_toolManager->ResetTools( TOOL_BASE::MODEL_RELOAD );

pcbnew/tools/drc_tool.cpp

@@ -36,7 +36,7 @@
 #include <drc/drc_engine.h>
 #include <drc/drc_results_provider.h>
 #include <netlist_reader/pcb_netlist.h>
-
+#include <dialogs/panel_setup_rules_base.h>
 
 DRC_TOOL::DRC_TOOL() :
         PCB_TOOL_BASE( "pcbnew.DRCTool" ),
@@ -59,7 +59,7 @@ void DRC_TOOL::Reset( RESET_REASON aReason )
     if( m_pcb != m_editFrame->GetBoard() )
     {
         if( m_drcDialog )
-            DestroyDRCDialog( wxID_OK );
+            DestroyDRCDialog();
 
         m_pcb = m_editFrame->GetBoard();
         m_drcEngine = m_pcb->GetDesignSettings().m_DRCEngine;
@@ -118,7 +118,7 @@ bool DRC_TOOL::IsDRCDialogShown()
 }
 
 
-void DRC_TOOL::DestroyDRCDialog( int aReason )
+void DRC_TOOL::DestroyDRCDialog()
 {
     if( m_drcDialog )
     {
@@ -134,6 +134,25 @@ void DRC_TOOL::RunTests( PROGRESS_REPORTER* aProgressReporter, bool aTestTracksA
     ZONE_FILLER_TOOL* zoneFiller = m_toolMgr->GetTool<ZONE_FILLER_TOOL>();
     BOARD_COMMIT      commit( m_editFrame );
     NETLIST           netlist;
+    wxWindowDisabler  disabler( /* disable everything except: */ m_drcDialog );
+
+    // This is not the time to have stale or buggy rules.  Ensure they're up-to-date
+    // and that they at least parse.
+    try
+    {
+        m_drcEngine->InitEngine( m_editFrame->Prj().AbsolutePath( "drc-rules" ) );
+    }
+    catch( PARSE_ERROR& pe )
+    {
+        // Shouldn't be necessary, but we get into all kinds of wxWidgets window ordering
+        // issues (on at least OSX) if we don't.
+        DestroyDRCDialog();
+
+        m_editFrame->ShowBoardSetupDialog( _( "Rules" ), pe.What(), ID_RULES_EDITOR,
+                                           pe.lineNumber, pe.byteIndex );
+
+        return;
+    }
 
     if( aRefillZones )
     {
@@ -148,9 +167,6 @@ void DRC_TOOL::RunTests( PROGRESS_REPORTER* aProgressReporter, bool aTestTracksA
         zoneFiller->CheckAllZones( m_drcDialog, aProgressReporter );
     }
 
-    // Re-initialize the DRC_ENGINE to make doubly sure everything is up-to-date
-    //
-    m_drcEngine->InitEngine( m_editFrame->Prj().AbsolutePath( "drc-rules" ) );
     m_drcEngine->SetWorksheet( m_editFrame->GetCanvas()->GetWorksheet() );
 
     if( aTestFootprints && !Kiface().IsSingle() )
@@ -192,6 +208,11 @@ void DRC_TOOL::RunTests( PROGRESS_REPORTER* aProgressReporter, bool aTestTracksA
     m_drcEngine->SetProgressReporter( nullptr );
     m_drcEngine->ClearViolationHandler();
 
+    m_drcDialog->SetDrcRun();
+
+    if( !netlist.IsEmpty() )
+        m_drcDialog->SetFootprintTestsRun();
+
     commit.Push( _( "DRC" ), false );
 
     // update the m_drcDialog listboxes

pcbnew/tools/drc_tool.h

@@ -98,19 +98,12 @@ public:
     bool IsDRCDialogShown();
 
     /**
-     * Deletes this ui dialog box and zeros out its pointer to remember
+     * Closes and frees the DRC dialog.
-     * the state of the dialog's existence.
-     *
-     * @param aReason Indication of which button was clicked to cause the destruction.
-     * if aReason == wxID_OK, design parameters values which can be entered from the dialog
-     * will bbe saved in design parameters list
      */
-    void DestroyDRCDialog( int aReason );
+    void DestroyDRCDialog();
 
     /**
-     * Run all the tests specified with a previous call to
+     * Run the DRC tests.
-     * SetSettings()
-     * @param aMessages = a wxTextControl where to display some activity messages. Can be NULL
      */
     void RunTests( PROGRESS_REPORTER* aProgressReporter, bool aTestTracksAgainstZones,
                    bool aRefillZones, bool aReportAllTrackErrors, bool aTestFootprints );

pcbnew/tools/pcb_inspection_tool.cpp

@@ -32,6 +32,7 @@
 #include <profile.h>
 #include <dialogs/wx_html_report_box.h>
 #include <drc/drc_engine.h>
+#include <dialogs/panel_setup_rules_base.h>
 #include "pcb_inspection_tool.h"
 
 
@@ -186,7 +187,17 @@ void PCB_INSPECTION_TOOL::reportCopperClearance( PCB_LAYER_ID aLayer, BOARD_CONN
     r->Report( "" );
 
     DRC_ENGINE drcEngine( m_frame->GetBoard(), &m_frame->GetBoard()->GetDesignSettings() );
-    drcEngine.InitEngine( m_frame->Prj().AbsolutePath( "drc-rules" ) );
+
+    try
+    {
+        drcEngine.InitEngine( m_frame->Prj().AbsolutePath( "drc-rules" ) );
+    }
+    catch( PARSE_ERROR& pe )
+    {
+        m_frame->ShowBoardSetupDialog( _( "Rules" ), pe.What(), ID_RULES_EDITOR,
+                                       pe.lineNumber, pe.byteIndex );
+        return;
+    }
 
     auto constraint = drcEngine.EvalRulesForItems( DRC_CONSTRAINT_TYPE_CLEARANCE, aA, aB,
                                                    aLayer, r );